diff --git a/CHANGELOG.md b/CHANGELOG.md index b81d87bf..63848388 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,8 +56,20 @@ finalized in place with a date — no renaming/migration step needed. - **Claude Code web-guard hook is topic/mount-scoped.** Only queries about the mounted products reach the web guard, and the retry cache is keyed per mount. +- **`codesearch serve --model X` sets the default model for newly created indexes.** Previously `--model` was inert on `serve` (each repo's query model comes from its own `metadata.json`); now it is the model a repo is indexed with when it is added through serve without an explicit model — `POST /repos`, including the `codesearch index add` path delegated to a running serve — so an operator can make a non-default model the norm without passing `--model` on every add. An index that already records its own model is never overridden (an explicit `model` in the request still wins). Serve reports the default in `GET /status` as `default_model` and at startup. It is deliberately **not** the query fallback for a repo whose `metadata.json` records no model: a legacy index built before the model-recording contract is queried with the built-in 384-dim default, and the search response carries a warning naming the assumed model and the re-index command. Following the serve default there would break a legacy repo the instant an operator set `--model` to a model of another dimension (a 384-dim index queried with a 768-dim model fails), and degrade it silently for a same-dimension model. + ### Fixed +- **`status` no longer reports `ready` for an index that cannot be searched.** Readiness keyed only off `total_chunks`, so a repo mid-rebuild — chunks inserted, `build_index()` not yet run — reported `status: "ready"` / "Index is ready for searching." while every search failed with `Index not built. Call build_index() after inserting chunks.` Both the single-repo and group status paths now require the vector index (`stats.indexed`) to be built before reporting `ready`, otherwise they report `building` with a message that says the vector index is not built. A store that failed to report stats is still surfaced separately (degraded-ready with `warnings`), never misread as "not built". + +- **Adding a repo with `--model` now creates the index at that model's dimension.** `POST /repos` — the path `codesearch index add --model …` delegates to when serve is running — opened the store with the default 384-dim dimension and applied the model override only to `metadata.json` afterwards. The background reindex then embedded 768-dim EmbeddingGemma vectors into a 384-dim store and indexed nothing: the `.codesearch.db` directory existed, `stats` showed the new dimension, and no files were indexed. The store is now opened at the override's dimension. + +- **The CLI no longer downgrades a non-default index to the 384-dim default.** `codesearch index` resolved its embedding model as `--model`-or-`ModelType::default()` and never consulted the model recorded in the index's `metadata.json`, so re-indexing a repo built with EmbeddingGemma embedded 384-dim MiniLM vectors against a 768-dim store (and `FileMetaStore` logged "Model changed, full re-index required", wiping the file metadata). The CLI now resolves the recorded model through the same helper the serve/watcher paths use; an explicit `--model` that disagrees is rejected with a pointer to `--force`. Relatedly, `codesearch stats`, `get_db_stats` and the repo listing opened the vector store with a hardcoded 384, so `Dimensions:` always read 384 for every index — they now read the recorded dimensions. + +- **`status(kind="index")` reports the routed repo's model, not the service default.** The `model` field was the service's own model (the hardcoded default in serve mode) while `dimensions` came from live store stats, so every repo read `minilm-l6-q` regardless of what it was indexed with. It now resolves per repo for `project=`, and reports the common model — or `mixed` — for a group. + +- **Serve mode embeds each query with its target repo's indexed model.** The multi-repo MCP service built its shared embedder from `ModelType::default()` (384-dim MiniLM) and ignored both `--model` and the `model_short_name` each index records, so every semantic query against an index rebuilt with a 768-dim model (EmbeddingGemma) failed with `Query embedding dimension mismatch: expected 768, got 384` — and a same-dimension mismatch would have silently compared incomparable vector spaces. The service now resolves the model per routed repo (the same `metadata.json` contract the indexing path already followed) through a per-model `EmbeddingServicePool`; group fan-out embeds the query once per distinct model and searches each store with its own. (See the `serve --model` default for newly created indexes under **Added**.) + - **Persisted helper-exit warnings are platform-stable — Linux CI green again.** `ExitStatus`'s `Display` renders `exit code: N` on Windows but `exit status: N` on Unix, so the non-zero-exit warning the TypeScript symbol indexer persists into its meta table disagreed with its own regression test on Linux, failing `test-linux`/`csharp-integration-tests` deterministically since #238. A shared `exit_status_text()` renders `exit code: N` on every platform (signal-terminated processes fall back to the platform string), applied to the persisted TS warning and the C#/TS helper log lines, pinned by a cross-platform unit test. - **C# canonical symbol keys no longer collapse distinct declarations.** Generic arity (``M`1``), the full containing-type chain and fully qualified parameter types are part of the key again, so overloads that previously shared one identity keep their own. The index version is bumped to 2.0 with a key-format stamp in the index meta: a stale-format index reports as absent and rebuilds once on upgrade (todo #139). diff --git a/README.md b/README.md index f18acdd4..a09b30db 100644 --- a/README.md +++ b/README.md @@ -132,6 +132,21 @@ spaces are model-specific. Keep the same model selected for later indexing runs. Search rejects a `--model` value that differs from the indexed model and points to the required `--force` rebuild instead of mixing incompatible vector spaces. +In **serve** mode the model is resolved **per repository**, from each index's own +metadata, not from a hub-wide setting: a hub may hold indexes built with +different models, and every query is embedded with the model of the repo it +targets (mixed-model groups are fine). `codesearch serve --model ` sets a +**default for newly created indexes**: a repo added without an explicit model +(e.g. `codesearch index add` with no `--model`, delegated to serve) is indexed +with it, and it is reported in `GET /status` as `default_model`. It never +overrides an index that already records its own model — to change an existing +repo's model, re-index that repo +(`codesearch --model index --force`) and restart serve. A repo +whose `metadata.json` records no model (a legacy index built before the +recording contract) is queried with the built-in 384-dim default rather than the +serve default, and the search response carries a warning naming the assumed model +and the re-index command. + ## MCP Configuration codesearch connects to AI agents via MCP. Two modes: diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 3a42da1f..0b64ced8 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -261,7 +261,9 @@ pub struct Cli { #[arg(long, global = true)] pub store: Option, - /// Embedding model to use (e.g., bge-small, jina-code, embeddinggemma-q4) + /// Embedding model to use (e.g., bge-small, jina-code, embeddinggemma-q4). + /// On `serve`, this is the default for newly created indexes; each repo's + /// existing index keeps the model recorded in its own metadata. #[arg(long, global = true)] pub model: Option, } @@ -1222,10 +1224,19 @@ pub async fn run(cancel_token: CancellationToken) -> Result<()> { if let Err(e) = crate::logger::init_serve_logger(log_level, effective_quiet) { eprintln!("Warning: failed to initialize serve logger: {}", e); } + // `--model` is a global flag inherited by every subcommand. On + // `serve` it sets the serve-wide default for newly created + // indexes — each repo's queries still use the model recorded + // in its own index metadata (a hub may mix models), so this + // never overrides an existing index. + if let Some(mt) = model_type { + warn_if_heavier_model(mt); + } crate::serve::run_serve( host, port, register, + model_type, no_tui, keep_warm_url, idle_suspend_secs, diff --git a/src/embed/embedder.rs b/src/embed/embedder.rs index 6372cf51..d0c2f606 100644 --- a/src/embed/embedder.rs +++ b/src/embed/embedder.rs @@ -5,7 +5,7 @@ use ort::ep::CPU; use crate::file::Language; /// Available embedding models -#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Default)] pub enum ModelType { // === MiniLM Family === /// All-MiniLM-L6-v2 - 384 dimensions, fast and efficient @@ -242,6 +242,24 @@ impl ModelType { } } + /// Resolve the embedding model recorded in an index's `metadata.json`. + /// + /// The reader counterpart to [`Self::write_metadata_fields`]. Returns `None` + /// when the file is missing/unreadable, carries no `model_short_name`, or + /// names a model this build does not know — callers fall back to + /// [`ModelType::default`]. Every query path MUST resolve the model per + /// target index through here (instead of assuming the default): embedding a + /// query with any model other than the one the index was built with either + /// fails with a dimension mismatch or silently compares incomparable vector + /// spaces. Multi-repo serve mode is where this matters most, because one + /// hub can hold indexes built with different models. + pub fn from_index_metadata(db_path: &std::path::Path) -> Option { + let content = std::fs::read_to_string(db_path.join("metadata.json")).ok()?; + let json: serde_json::Value = serde_json::from_str(&content).ok()?; + let name = json.get("model_short_name").and_then(|v| v.as_str())?; + Self::parse(name) + } + pub fn prepare_query(&self, text: &str) -> String { match self { Self::EmbeddingGemma300MQ4 => format!("task: search result | query: {text}"), diff --git a/src/embed/mod.rs b/src/embed/mod.rs index 717ed077..bc0a497f 100644 --- a/src/embed/mod.rs +++ b/src/embed/mod.rs @@ -10,6 +10,7 @@ pub use cache::{ pub use embedder::{FastEmbedder, ModelType}; use anyhow::Result; +use std::collections::HashMap; use std::env; use std::sync::{Arc, Mutex}; @@ -298,6 +299,50 @@ impl Default for EmbeddingService { } } +/// Lazily-created, per-model cache of [`EmbeddingService`]s. +/// +/// Serve mode is multi-repo and different repos may be indexed with different +/// embedding models (an older MiniLM index alongside a rebuilt EmbeddingGemma +/// one), so a single shared service is wrong: the query must be embedded with +/// the same model the target index was built with. The pool loads each model at +/// most once per serve instance and reuses it across MCP sessions and REST +/// handlers. Each model gets its own mutex, so queries against different models +/// do not serialise on one global lock. +#[derive(Default)] +pub struct EmbeddingServicePool { + services: Mutex>>>, + cache_dir: Option, +} + +impl EmbeddingServicePool { + /// Create a pool. `cache_dir` overrides the ONNX model cache directory + /// (`None` = fastembed's configured cache, i.e. the global models dir). + pub fn new(cache_dir: Option) -> Self { + Self { + services: Mutex::new(HashMap::new()), + cache_dir, + } + } + + /// Return the service for `model`, loading its ONNX model on first use. + /// + /// The returned `Arc` is locked independently per model, so a caller can + /// hold it across an `embed_query` without blocking other models. + pub fn get(&self, model: ModelType) -> Result>> { + let mut guard = self + .services + .lock() + .map_err(|e| anyhow::anyhow!("Embedding service pool mutex poisoned: {e}"))?; + if let Some(existing) = guard.get(&model) { + return Ok(existing.clone()); + } + let service = EmbeddingService::with_cache_dir(model, self.cache_dir.as_deref())?; + let arc = Arc::new(Mutex::new(service)); + guard.insert(model, arc.clone()); + Ok(arc) + } +} + #[cfg(test)] mod tests { use super::*; @@ -308,6 +353,50 @@ mod tests { assert_eq!(model.dimensions(), 384); } + /// The index-metadata reader must invert `write_metadata_fields`, and must + /// report "no answer" (None) for unknown/missing names rather than silently + /// claiming the default — callers decide the fallback. + #[test] + fn test_model_type_round_trips_through_index_metadata() { + for model in [ + ModelType::AllMiniLML6V2Q, + ModelType::EmbeddingGemma300MQ4, + ModelType::BGEBaseENV15, + ] { + let dir = tempfile::tempdir().unwrap(); + let mut obj = serde_json::Map::new(); + model.write_metadata_fields(&mut obj); + std::fs::write( + dir.path().join("metadata.json"), + serde_json::to_string(&obj).unwrap(), + ) + .unwrap(); + assert_eq!( + ModelType::from_index_metadata(dir.path()), + Some(model), + "reader must invert the writer for '{:?}'", + model + ); + } + + let dir = tempfile::tempdir().unwrap(); + std::fs::write( + dir.path().join("metadata.json"), + r#"{"model_short_name":"not-a-real-model"}"#, + ) + .unwrap(); + assert_eq!(ModelType::from_index_metadata(dir.path()), None); + + let dir = tempfile::tempdir().unwrap(); + std::fs::write(dir.path().join("metadata.json"), "{}").unwrap(); + assert_eq!(ModelType::from_index_metadata(dir.path()), None); + + assert_eq!( + ModelType::from_index_metadata(std::path::Path::new("/nonexistent-db-dir")), + None + ); + } + #[test] #[ignore] // Requires model download fn test_embedding_service_creation() { diff --git a/src/index/manager.rs b/src/index/manager.rs index 23412cb1..c0d54bc4 100644 --- a/src/index/manager.rs +++ b/src/index/manager.rs @@ -504,7 +504,7 @@ impl IndexManager { /// /// Fails fast if metadata is missing, names an unknown model, or records a /// dimension count that disagrees with the resolved model. - fn resolve_embed_model(db_path: &Path) -> Result<(ModelType, usize)> { + pub(crate) fn resolve_embed_model(db_path: &Path) -> Result<(ModelType, usize)> { let metadata_path = db_path.join("metadata.json"); if !metadata_path.exists() { return Err(anyhow::anyhow!( diff --git a/src/index/mod.rs b/src/index/mod.rs index 148e47f2..273c460a 100644 --- a/src/index/mod.rs +++ b/src/index/mod.rs @@ -47,6 +47,58 @@ pub(crate) fn ensure_hnsw_index_if_needed( } } +/// Dimensions recorded in an index's `metadata.json` (fallback: the default). +/// +/// The vector store MUST be opened with the dimensions the index was built +/// with. `codesearch stats` / `get_db_stats` used to pass a hardcoded 384, so +/// they reported `Dimensions: 384` for every index — including 768-dim +/// EmbeddingGemma ones — and opened those stores with the wrong dimension. +fn recorded_dimensions(db_path: &Path) -> usize { + let from_metadata = std::fs::read_to_string(db_path.join("metadata.json")) + .ok() + .and_then(|c| serde_json::from_str::(&c).ok()) + .and_then(|j| j.get("dimensions").and_then(|v| v.as_u64())) + .map(|d| d as usize); + from_metadata + .or_else(|| ModelType::from_index_metadata(db_path).map(|m| m.dimensions())) + .unwrap_or(crate::constants::DEFAULT_EMBEDDING_DIMENSIONS) +} + +/// Resolve the embedding model for an indexing run. +/// +/// On an existing index the model recorded in `metadata.json` wins: embedding +/// with any other model (including the hardcoded default) either fails with a +/// dimension mismatch or silently mixes vector spaces. An explicit `--model` +/// that disagrees with the recorded model is rejected — the index must be +/// rebuilt with `--force` to change models. +/// +/// This mirrors `IndexManager::resolve_embed_model`, which the serve and +/// watcher paths already use; the CLI `index` path used `model.unwrap_or_default()` +/// and therefore downgraded any non-default index to 384-dim MiniLM. +fn resolve_index_model( + db_path: &Path, + force: bool, + requested: Option, +) -> Result { + // `--force` deletes the database (see `get_db_path_smart`), so there is no + // recorded model to honour; a non-existent/legacy index has none either. + if force || !db_path.join("metadata.json").exists() { + return Ok(requested.unwrap_or_default()); + } + let (recorded, _dims) = IndexManager::resolve_embed_model(db_path)?; + match requested { + Some(req) if req != recorded => Err(anyhow::anyhow!( + "Model mismatch: {} was indexed with '{}', but --model '{}' was requested.\n\ + To change models, rebuild the index: codesearch --model {} index --force", + db_path.display(), + recorded.short_name(), + req.short_name(), + req.short_name() + )), + _ => Ok(recorded), + } +} + /// Update metadata.json with current chunk/file counts so that `status(projects)` /// can report accurate numbers without opening LMDB. /// Uses atomic read-modify-write (temp+rename) so a crash never leaves an empty file. @@ -549,7 +601,11 @@ async fn index_with_options( cancel_token: CancellationToken, ) -> Result<()> { let (db_path, project_path) = get_db_path_smart(path, global, force)?; - let model_type = model.unwrap_or_default(); + // Resolve the embedding model BEFORE touching the index: on an existing + // index the model recorded in metadata.json wins, and an explicit `--model` + // that disagrees is rejected (rebuild with --force). Defaulting here would + // embed 384-dim MiniLM vectors into a 768-dim index. See `resolve_index_model`. + let model_type = resolve_index_model(&db_path, force, model)?; // Macro to conditionally print macro_rules! log_print { @@ -714,7 +770,7 @@ async fn index_with_options( if total_chunks_to_delete > 0 { log_print!("\n🔄 Deleting {} old chunks...", total_chunks_to_delete); - let mut store = VectorStore::new(&db_path, 384)?; // Will load dimensions from DB + let mut store = VectorStore::new(&db_path, model_type.dimensions())?; let mut fts_store = FtsStore::new_with_writer(&db_path)?; // Delete deleted files' metadata and chunks @@ -1292,7 +1348,7 @@ pub async fn stats(path: Option) -> Result<()> { println!("💾 Database: {}", db_path.display()); println!("📂 Project: {}", project_path.display()); - let store = VectorStore::new(&db_path, 384)?; // We'll need to store dimensions in metadata + let store = VectorStore::new(&db_path, recorded_dimensions(&db_path))?; let stats = store.stats()?; println!("\n{}", "Vector Store:".bright_green()); @@ -1367,7 +1423,7 @@ fn print_repo_stats(repo_path: &Path, db_path: &Path) -> Result<()> { println!(" 📂 {}", repo_path.display()); // Try to load stats - match VectorStore::new(db_path, 384) { + match VectorStore::new(db_path, recorded_dimensions(db_path)) { Ok(store) => match store.stats() { Ok(stats) => { println!( @@ -1813,7 +1869,7 @@ async fn get_db_stats(db_path: &Path) -> Result { } // Try to get stats from vector store - let store = VectorStore::new(db_path, 384)?; + let store = VectorStore::new(db_path, recorded_dimensions(db_path))?; let stats = store.stats()?; // Calculate database size @@ -3275,3 +3331,112 @@ mod remove_order_tests { assert_global_config_unchanged(_canary); } } + +/// Model resolution for the CLI `index` path: the model recorded in the index's +/// `metadata.json` must win, so re-indexing a non-default index (e.g. rebuilt +/// with EmbeddingGemma) does not silently downgrade it to 384-dim MiniLM. +/// +/// Regression guard for `index_with_options` using `model.unwrap_or_default()`: +/// with the defect reintroduced, `resolve_index_model` returns the default for +/// the gemma case below and the test fails. +#[cfg(test)] +mod index_model_resolution_tests { + use super::*; + + /// Write an index metadata.json recording `model` (as the indexer does). + fn write_metadata(db_path: &Path, model: ModelType) { + std::fs::create_dir_all(db_path).unwrap(); + let mut obj = serde_json::Map::new(); + model.write_metadata_fields(&mut obj); + std::fs::write( + db_path.join("metadata.json"), + serde_json::to_string(&obj).unwrap(), + ) + .unwrap(); + } + + #[test] + fn recorded_dimensions_reads_metadata_dimensions() { + let dir = tempfile::tempdir().unwrap(); + let db = dir.path().join(".codesearch.db"); + write_metadata(&db, ModelType::EmbeddingGemma300MQ4); + assert_eq!(recorded_dimensions(&db), 768); + } + + #[test] + fn recorded_dimensions_falls_back_to_model_then_default() { + // Dimensions key absent, model known -> model's dimensions. + let dir = tempfile::tempdir().unwrap(); + let db = dir.path().join(".codesearch.db"); + std::fs::create_dir_all(&db).unwrap(); + std::fs::write( + db.join("metadata.json"), + r#"{"model_short_name":"embeddinggemma-q4"}"#, + ) + .unwrap(); + assert_eq!(recorded_dimensions(&db), 768); + + // No metadata at all -> default. + let dir = tempfile::tempdir().unwrap(); + assert_eq!( + recorded_dimensions(&dir.path().join(".codesearch.db")), + crate::constants::DEFAULT_EMBEDDING_DIMENSIONS + ); + } + + #[test] + fn resolve_index_model_prefers_recorded_model_on_existing_index() { + // The defect: a bare `codesearch index` on a gemma index picked the + // default (384-dim MiniLM). The recorded model must win. + let dir = tempfile::tempdir().unwrap(); + let db = dir.path().join(".codesearch.db"); + write_metadata(&db, ModelType::EmbeddingGemma300MQ4); + + assert_eq!( + resolve_index_model(&db, false, None).unwrap(), + ModelType::EmbeddingGemma300MQ4, + "an existing index's recorded model must be used, not the default" + ); + // An explicit --model that agrees is accepted. + assert_eq!( + resolve_index_model(&db, false, Some(ModelType::EmbeddingGemma300MQ4)).unwrap(), + ModelType::EmbeddingGemma300MQ4 + ); + } + + #[test] + fn resolve_index_model_rejects_disagreeing_override_without_force() { + let dir = tempfile::tempdir().unwrap(); + let db = dir.path().join(".codesearch.db"); + write_metadata(&db, ModelType::EmbeddingGemma300MQ4); + + let err = resolve_index_model(&db, false, Some(ModelType::AllMiniLML6V2Q)) + .expect_err("a --model that disagrees with the index must be rejected"); + let msg = format!("{err:#}"); + assert!( + msg.contains("Model mismatch") && msg.contains("embeddinggemma-q4"), + "error must name the recorded model and the mismatch, got: {msg}" + ); + + // --force deletes the DB, so the requested model is honoured. + assert_eq!( + resolve_index_model(&db, true, Some(ModelType::AllMiniLML6V2Q)).unwrap(), + ModelType::AllMiniLML6V2Q + ); + } + + #[test] + fn resolve_index_model_uses_requested_model_for_a_fresh_index() { + let dir = tempfile::tempdir().unwrap(); + let db = dir.path().join(".codesearch.db"); // does not exist + + assert_eq!( + resolve_index_model(&db, false, Some(ModelType::EmbeddingGemma300MQ4)).unwrap(), + ModelType::EmbeddingGemma300MQ4 + ); + assert_eq!( + resolve_index_model(&db, false, None).unwrap(), + ModelType::default() + ); + } +} diff --git a/src/mcp/mod.rs b/src/mcp/mod.rs index 9dcd746a..66d79ef0 100644 --- a/src/mcp/mod.rs +++ b/src/mcp/mod.rs @@ -28,7 +28,7 @@ fn serve_url_from_env() -> String { } use crate::db_discovery::{find_best_database, load_repos_config}; -use crate::embed::{EmbeddingService, ModelType}; +use crate::embed::{EmbeddingServicePool, ModelType}; use crate::file::Language; use crate::fts::FtsStore; use crate::index::SharedStores; @@ -148,8 +148,10 @@ pub struct CodesearchService { project_path: PathBuf, model_type: ModelType, dimensions: usize, - // Lazily initialized on first search - embedding_service: Arc>>, + // Lazily initialized on first search. A per-model pool: serve mode is + // multi-repo and each index records the model it was built with, so the + // query model is resolved per target repo (see `query_model`). + embedding_pool: Arc, // Shared stores for concurrent access (optional - only set when running with IndexManager) shared_stores: Option>, // Serve-mode state (set when running inside `codesearch serve`) @@ -194,6 +196,18 @@ impl Drop for CodesearchService { } } +/// Outcome of resolving the embedding model for a query target. +/// +/// See [`CodesearchService::resolve_query_model`]. +pub(crate) struct QueryModel { + /// The model the query must be embedded with. + pub model: ModelType, + /// A caller-facing warning, set when the target index records no model and + /// the built-in default was assumed. `None` when the model was recorded or + /// the query is scope-free. + pub assumed_warning: Option, +} + // v1: supports prefix/suffix patterns with `*` and `**` only. /// Merge exact FTS results into the main result set, deduplicating by chunk_id /// and keeping the max score for duplicates. @@ -884,7 +898,9 @@ impl CodesearchService { project_path, model_type, dimensions, - embedding_service: Arc::new(Mutex::new(None)), + embedding_pool: Arc::new(EmbeddingServicePool::new( + crate::constants::get_global_models_cache_dir().ok(), + )), shared_stores, serve_state: None, symbol_registry: Arc::new(SymbolIndexerRegistry::new()), @@ -898,13 +914,19 @@ impl CodesearchService { /// it routes requests to the repo identified by `project`/`group`. pub(crate) fn new_for_serve(serve_state: Arc) -> Result { let symbol_registry = serve_state.symbol_registry(); + // Seed the service with the serve-wide default model (`serve --model`), + // the same value `POST /repos` stamps into a newly created index, so the + // scope-free status summary reports it. It is deliberately NOT the query + // fallback for a repo whose metadata records no model — that resolves to + // the built-in default, with a warning. See `resolve_query_model`. + let model_type = serve_state.default_model().unwrap_or_default(); Ok(Self { tool_router: Self::merged_tool_router(), db_path: PathBuf::from("serve://multi-repo"), project_path: PathBuf::from("serve://multi-repo"), - model_type: ModelType::default(), - dimensions: crate::constants::DEFAULT_EMBEDDING_DIMENSIONS, - embedding_service: serve_state.embedding_service(), + model_type, + dimensions: model_type.dimensions(), + embedding_pool: serve_state.embedding_pool(), shared_stores: None, serve_state: Some(serve_state), symbol_registry, @@ -923,17 +945,71 @@ impl CodesearchService { self.tracks_session = true; } - /// Get or initialize the embedding service - fn get_embedding_service(&self) -> Result>> { - let mut guard = self.embedding_service.lock().unwrap(); - if guard.is_none() { - let cache_dir = crate::constants::get_global_models_cache_dir()?; - *guard = Some(EmbeddingService::with_cache_dir( - self.model_type, - Some(&cache_dir), - )?); + /// Resolve the embedding model a query against `alias` must use. + /// + /// With a repo alias (`project=` / group member) the model is read from that + /// repo's index metadata — an index built with EmbeddingGemma must be + /// queried with EmbeddingGemma, not the 384-dim default. A repo whose + /// metadata records no model is queried with the built-in default, never the + /// serve-wide `--model` default (see [`Self::resolve_query_model`]). With no + /// alias this returns the service's own `model_type`: the local index + /// metadata in stdio mode, the serve default in serve mode (the scope-free + /// status summary). Prefer [`Self::resolve_query_model`] when the caller can + /// surface the unrecorded-model warning. + pub(crate) fn query_model(&self, alias: Option<&str>) -> ModelType { + self.resolve_query_model(alias).model + } + + /// Resolve the query model together with a warning when it had to be assumed. + /// + /// The model a query is embedded with must match the model the target index + /// was built with. When `alias`'s metadata records no model the model is + /// unknowable, so this assumes the BUILT-IN default: that is both the + /// historical 384-dim behaviour and the value every other reader assumes for + /// metadata without a `model_short_name`. It deliberately does NOT assume the + /// serve-wide `--model` default: that flag selects the model for newly + /// created indexes, and using it here would break a working legacy repo the + /// moment an operator set it (a 384-dim index queried with a 768-dim model + /// fails, and a same-dimension model degrades rankings silently). The + /// returned warning names the repo, the assumption, and the re-index command. + pub(crate) fn resolve_query_model(&self, alias: Option<&str>) -> QueryModel { + if let (Some(state), Some(alias)) = (self.serve_state.as_ref(), alias) { + if let Some(model) = state.model_for_alias(alias) { + return QueryModel { + model, + assumed_warning: None, + }; + } + let model = ModelType::default(); + let warning = format!( + "repo '{alias}' records no embedding model; queried with the built-in default '{}' ({} dims). If this repo was indexed with a different model, re-index it: codesearch index --force --model ", + model.short_name(), + model.dimensions() + ); + if state.mark_legacy_model_warned(alias) { + tracing::warn!("{}", warning); + } + return QueryModel { + model, + assumed_warning: Some(warning), + }; + } + QueryModel { + model: self.model_type, + assumed_warning: None, } - Ok(guard) + } + + /// Get (lazily initializing) the embedding service for `model`. + /// + /// The returned `Arc>` is per-model, so concurrent queries against + /// different models do not serialise on one global lock. Callers MUST pass + /// the model the target index was built with — see [`Self::query_model`]. + pub(crate) fn embedding_service_for( + &self, + model: ModelType, + ) -> Result>> { + self.embedding_pool.get(model) } /// Return the current MCP mode as a string for diagnostics. @@ -1247,8 +1323,11 @@ impl CodesearchService { /// Fan-out vector store read across multiple stores, merging results. /// - /// Runs `action` against each store and merges all results into a single vec, - /// deduplicating by (alias, chunk_id) (keeping highest score) and sorting by score descending. + /// Runs `action(alias, store)` against each store and merges all results into + /// a single vec, deduplicating by (alias, chunk_id) (keeping highest score) + /// and sorting by score descending. The `alias` is passed to the closure so + /// callers can select per-repo state — notably the query embedding for that + /// repo's own model (see `semantic_search_multi`). /// /// A per-store failure does NOT abort the fan-out — one broken repo should /// not blind a group query to the healthy ones — but it is reported back in @@ -1261,7 +1340,7 @@ impl CodesearchService { aliases: &[String], ) -> Result> where - F: FnMut(&VectorStore) -> anyhow::Result>, + F: FnMut(&str, &VectorStore) -> anyhow::Result>, R: Clone + HasChunkId + HasScore, { let mut failures: Vec<(String, String)> = Vec::new(); @@ -1272,7 +1351,7 @@ impl CodesearchService { for (idx, store_arc) in stores.iter().enumerate() { let alias = aliases.get(idx).map(|s| s.as_str()).unwrap_or("unknown"); let store = store_arc.vector_store.read().await; - match action(&store) { + match action(alias, &store) { Ok(results) => { for r in results { let key = (alias.to_string(), r.chunk_id()); diff --git a/src/mcp/responses.rs b/src/mcp/responses.rs index 9ad43720..4507fef3 100644 --- a/src/mcp/responses.rs +++ b/src/mcp/responses.rs @@ -181,6 +181,7 @@ pub(crate) fn index_status_summary( total_repos: usize, failed_count: usize, total_chunks: usize, + all_indexed: bool, ) -> (String, String) { if total_repos > 0 && failed_count >= total_repos { ( @@ -196,6 +197,17 @@ pub(crate) fn index_status_summary( "Index is being built across {total_repos} repo(s). Searches may fail until indexing completes." ), ) + } else if !all_indexed { + // Chunks are in the stores but the HNSW vector index is not built yet + // (`VectorStore::search` refuses without it). Reporting "ready" here — + // the old behaviour, which keyed only off `total_chunks` — told an + // operator a rebuild was finished while every search still failed. + ( + "building".to_string(), + format!( + "Chunks are indexed across {total_repos} repo(s) but the vector index is not built yet — searches may fail until indexing completes." + ), + ) } else if failed_count > 0 { ( "ready".to_string(), @@ -212,6 +224,32 @@ pub(crate) fn index_status_summary( } } +/// Status/message for a single routed store's index. +/// +/// `indexed` (the HNSW graph is built and committed) is load-bearing: a store +/// with chunks but no built graph is NOT searchable — `VectorStore::search` +/// fails with "Index not built" — so it must not read as `ready`. During a +/// rebuild there is a window where chunks are inserted but `build_index()` has +/// not run yet, which is exactly when an operator asks "is the migration done?". +pub(crate) fn single_index_status(total_chunks: usize, indexed: bool) -> (String, String) { + if total_chunks == 0 { + ( + "building".to_string(), + "Index is being built in the background. Searches may fail until indexing completes. Please check back in a few minutes.".to_string(), + ) + } else if !indexed { + ( + "building".to_string(), + "Chunks are indexed but the vector index is not built yet — searches may fail until indexing completes. Please check back in a few minutes.".to_string(), + ) + } else { + ( + "ready".to_string(), + "Index is ready for searching.".to_string(), + ) + } +} + /// Turn a store's `stats()` result into the `(total_chunks, total_files, /// error)` triple `list_projects` reports per repo. /// diff --git a/src/mcp/search.rs b/src/mcp/search.rs index f15dc9c7..a00499a9 100644 --- a/src/mcp/search.rs +++ b/src/mcp/search.rs @@ -172,9 +172,16 @@ impl CodesearchService { } // === Modes: "semantic", "hybrid", "auto" — require embedding === + // The query MUST be embedded with the model the target index was built + // with. In serve mode that is the routed repo's recorded model, not a + // hub-wide default: a 384-dim query against a 768-dim EmbeddingGemma + // index failed with "expected 768, got 384". A repo that records no + // model is queried with the built-in default, and the caller is warned. + let model_resolution = self.resolve_query_model(ctx.project_alias.as_deref()); let query_embedding = { - let mut service_guard = match self.get_embedding_service() { - Ok(g) => g, + let model = model_resolution.model; + let service = match self.embedding_service_for(model) { + Ok(s) => s, Err(e) => { tracing::error!("MCP: Failed to get embedding service: {:?}", e); return Ok(CallToolResult::success(vec![ContentBlock::text(format!( @@ -183,8 +190,11 @@ impl CodesearchService { } }; - let service = service_guard.as_mut().unwrap(); - tracing::debug!("MCP: Embedding query..."); + let mut service = service.lock().unwrap(); + tracing::debug!( + "MCP: Embedding query with model '{}'...", + model.short_name() + ); match service.embed_query(&request.query) { Ok(e) => e, Err(e) => { @@ -201,6 +211,11 @@ impl CodesearchService { // here, `project=` — the form an agent uses most — still reports // a broken store as an ordinary empty result. let mut single_warnings: Vec = Vec::new(); + // Surface the assumed-model warning even when the store read succeeds: + // mismatched vector spaces do not error, they just rank wrongly. + if let Some(warning) = model_resolution.assumed_warning { + single_warnings.push(warning); + } // Search vector store let vector_results = match self @@ -567,32 +582,69 @@ impl CodesearchService { } // === Modes requiring embedding: "semantic", "hybrid", "auto" === - let query_embedding = { - let mut service_guard = match self.get_embedding_service() { - Ok(g) => g, - Err(e) => { - return Ok(CallToolResult::success(vec![ContentBlock::text(format!( - "Error initializing embedding service: {e:#}" - ))])); - } - }; - let service = service_guard.as_mut().unwrap(); - match service.embed_query(&request.query) { - Ok(e) => e, - Err(e) => { - return Ok(CallToolResult::success(vec![ContentBlock::text(format!( - "Error embedding query: {e:#}" - ))])); + // + // Each repo may have been indexed with a different model, so the query + // is embedded once per distinct model and every store is searched with + // the embedding of ITS model. Embedding all stores with one hub-wide + // default is what produced "Query embedding dimension mismatch: + // expected 768, got 384" on a mixed hub. + let mut embeddings_by_alias: std::collections::HashMap> = + std::collections::HashMap::with_capacity(aliases.len()); + // Assumed-model warnings, one per repo that records no model. Collected + // here and folded into `search_warnings` below so an agent sees the + // assumption alongside the results it applies to. + let mut model_warnings: Vec = Vec::new(); + { + let mut by_model: std::collections::HashMap> = + std::collections::HashMap::new(); + for alias in aliases { + let model_resolution = self.resolve_query_model(Some(alias)); + let model = model_resolution.model; + if let Some(warning) = model_resolution.assumed_warning { + model_warnings.push(warning); } + let embedding = match by_model.get(&model) { + Some(cached) => cached.clone(), + None => { + let service = match self.embedding_service_for(model) { + Ok(s) => s, + Err(e) => { + return Ok(CallToolResult::success(vec![ContentBlock::text( + format!( + "Error initializing embedding service for '{alias}': {e:#}" + ), + )])); + } + }; + let mut service = service.lock().unwrap(); + let embedding = match service.embed_query(&request.query) { + Ok(e) => e, + Err(e) => { + return Ok(CallToolResult::success(vec![ContentBlock::text( + format!("Error embedding query: {e:#}"), + )])); + } + }; + by_model.insert(model, embedding.clone()); + embedding + } + }; + embeddings_by_alias.insert(alias.clone(), embedding); } - }; + } - // Search vector stores across all repos + // Search vector stores across all repos, each with its own model's + // query embedding. let outcome = self .with_vector_store_read_multi( - |store| { + |alias, store| { + let embedding = embeddings_by_alias.get(alias).ok_or_else(|| { + anyhow::anyhow!( + "internal error: no query embedding resolved for repo '{alias}'" + ) + })?; store - .search(&query_embedding, limit * 5) + .search(embedding, limit * 5) .context("Error searching vector store") }, stores.clone(), @@ -602,7 +654,8 @@ impl CodesearchService { // Warnings raised by the fan-out, carried into the response so the // calling agent can tell "not in the corpus" from "that repo is down". - let mut search_warnings: Vec = Vec::new(); + // Seeded with any assumed-model warnings gathered while embedding. + let mut search_warnings: Vec = model_warnings; let vector_results = match outcome { diff --git a/src/mcp/status.rs b/src/mcp/status.rs index 2b0cfdda..969f3d4c 100644 --- a/src/mcp/status.rs +++ b/src/mcp/status.rs @@ -32,6 +32,29 @@ impl CodesearchService { } } + /// Model label for a grouped index-status response: the common model when + /// every member agrees, `"mixed"` when a hub holds indexes built with + /// different models. + /// + /// The status `model` field used to be the service's own model — the + /// hardcoded default in serve mode — so every repo read as `minilm-l6-q` + /// even when indexed with EmbeddingGemma. See the serve query-model fix. + pub(crate) fn group_model_label(&self, aliases: &[String]) -> String { + let mut common: Option = None; + for alias in aliases { + let model = self.query_model(Some(alias)); + match common { + None => common = Some(model), + Some(prev) if prev != model => return "mixed".to_string(), + _ => {} + } + } + common + .unwrap_or_else(|| self.query_model(None)) + .short_name() + .to_string() + } + // ───────────────────────────────────────────────────────────────── /// Internal implementation for index_status with optional project/group routing. async fn index_status_impl( @@ -132,6 +155,11 @@ impl CodesearchService { let mut max_chunk_id = 0u32; let mut dimensions = 0usize; let mut all_indexed = true; + // Separate from `all_indexed`: a store that FAILED to report stats + // must not make the summary claim "not built" (the failure already + // rides the `warnings` channel). This tracks only the graph state of + // stores that answered. + let mut all_built = true; let aliases = ctx.aliases(); let mut stats_warnings: Vec = Vec::new(); let mut failed_count = 0usize; @@ -150,6 +178,7 @@ impl CodesearchService { } if !stats.indexed { all_indexed = false; + all_built = false; } } // `all_indexed = false` alone renders identically to "still @@ -166,7 +195,7 @@ impl CodesearchService { } let (status, status_message) = - index_status_summary(sv.len(), failed_count, total_chunks); + index_status_summary(sv.len(), failed_count, total_chunks, all_built); let response = IndexStatusResponse { indexed: all_indexed, @@ -174,7 +203,7 @@ impl CodesearchService { status_message, total_chunks, total_files, - model: self.model_type.short_name().to_string(), + model: self.group_model_label(ctx.aliases()), dimensions, max_chunk_id, db_path: format!("({} repos)", sv.len()), @@ -202,7 +231,10 @@ impl CodesearchService { status_message: format!("{}", e), total_chunks: 0, total_files: 0, - model: self.model_type.short_name().to_string(), + model: self + .query_model(ctx.project_alias.as_deref()) + .short_name() + .to_string(), dimensions: 0, max_chunk_id: 0, db_path: self.db_path.display().to_string(), @@ -215,18 +247,9 @@ impl CodesearchService { } }; - // Determine status based on database state - let (status, status_message) = if stats.total_chunks == 0 { - ( - "building".to_string(), - "Index is being built in the background. Searches may fail until indexing completes. Please check back in a few minutes.".to_string(), - ) - } else { - ( - "ready".to_string(), - "Index is ready for searching.".to_string(), - ) - }; + // Determine status based on database state. `stats.indexed` (the HNSW + // graph is built) is load-bearing — see `single_index_status`. + let (status, status_message) = single_index_status(stats.total_chunks, stats.indexed); let response = IndexStatusResponse { indexed: stats.indexed, @@ -234,7 +257,10 @@ impl CodesearchService { status_message, total_chunks: stats.total_chunks, total_files: stats.total_files, - model: self.model_type.short_name().to_string(), + model: self + .query_model(ctx.project_alias.as_deref()) + .short_name() + .to_string(), dimensions: stats.dimensions, max_chunk_id: stats.max_chunk_id, db_path: self.db_path.display().to_string(), diff --git a/src/mcp/tests.rs b/src/mcp/tests.rs index 033888f1..3495b472 100644 --- a/src/mcp/tests.rs +++ b/src/mcp/tests.rs @@ -2044,14 +2044,14 @@ fn note_store_failure_survives_a_short_alias_list() { #[test] fn index_status_summary_reports_building_before_anything_failed() { - let (status, message) = super::index_status_summary(3, 0, 0); + let (status, message) = super::index_status_summary(3, 0, 0, true); assert_eq!(status, "building"); assert!(!message.contains("failed"), "got: {message}"); } #[test] fn index_status_summary_reports_clean_ready_with_no_failures() { - let (status, message) = super::index_status_summary(3, 0, 500); + let (status, message) = super::index_status_summary(3, 0, 500, true); assert_eq!(status, "ready"); assert!(!message.contains("failed"), "got: {message}"); assert!(message.contains("3 repo(s)"), "got: {message}"); @@ -2061,7 +2061,7 @@ fn index_status_summary_reports_clean_ready_with_no_failures() { fn index_status_summary_surfaces_a_degraded_group_as_ready_with_a_count() { // This is the exact case that used to be indistinguishable from // "index still warming": some data is in, one store didn't answer. - let (status, message) = super::index_status_summary(3, 1, 500); + let (status, message) = super::index_status_summary(3, 1, 500, true); assert_eq!( status, "ready", "the two healthy stores must not be masked by the one that failed" @@ -2081,7 +2081,7 @@ fn index_status_summary_reports_error_when_every_store_failed() { // this fix, `total_chunks == 0` was checked first and this rendered as // "building" — byte-identical to "not indexed yet" — even though every // store actively failed. `failed_count >= total_repos` must win. - let (status, message) = super::index_status_summary(3, 3, 0); + let (status, message) = super::index_status_summary(3, 3, 0, true); assert_eq!( status, "error", "a group where every store failed must not read as merely 'still building'" @@ -2765,3 +2765,46 @@ fn readonly_or_require_ready_never_creates_a_missing_index() { assert!(!super::may_create_missing_index(true, true, false)); assert!(!super::may_create_missing_index(true, false, true)); } + +/// Chunks without a built vector index are NOT searchable, and must not read as +/// `ready`. Regression guard for a rebuild window (and the mixed-state hub the +/// model migration exposed): `total_chunks > 0` used to be sufficient for +/// "ready", so `status` said "Index is ready for searching" while every search +/// failed with "Index not built. Call build_index() after inserting chunks." +#[test] +fn index_status_summary_reports_building_when_chunks_exist_but_graph_is_not_built() { + let (status, message) = super::index_status_summary(3, 0, 500, false); + assert_eq!( + status, "building", + "chunks without a built vector index are not searchable" + ); + assert!(message.contains("not built"), "got: {message}"); + assert!( + message.contains("3 repo(s)"), + "message must still name the scope, got: {message}" + ); + // A stats() failure on some stores must NOT be misread as "not built". + let (status, _) = super::index_status_summary(3, 1, 500, true); + assert_eq!( + status, "ready", + "a failed store is surfaced as degraded-ready" + ); +} + +/// Single-store counterpart: the same `total_chunks > 0` shortcut reported +/// `ready` for a store whose graph was never built (`indexed == false`). +#[test] +fn single_index_status_requires_a_built_graph_to_report_ready() { + let (status, message) = super::single_index_status(403, false); + assert_eq!( + status, "building", + "403 chunks with no built graph are not searchable" + ); + assert!(message.contains("not built"), "got: {message}"); + + let (status, _) = super::single_index_status(403, true); + assert_eq!(status, "ready"); + + let (status, _) = super::single_index_status(0, false); + assert_eq!(status, "building"); +} diff --git a/src/serve/mod.rs b/src/serve/mod.rs index ea27db53..c1b43f6b 100644 --- a/src/serve/mod.rs +++ b/src/serve/mod.rs @@ -269,12 +269,33 @@ pub(crate) struct ServeState { /// `find_impact` to reuse helper-detection cache instead of creating fresh /// instances per request. symbol_registry: Arc, - /// Shared embedding service — used by MCP sessions AND the REST handlers so - /// the ONNX embedding model is loaded ONCE per serve instance (lazily, on - /// the first semantic query) and reused across all requests. Without this, - /// per-request `CodesearchService` construction (REST handlers) would reload - /// the model on every call (~100ms–2s). Mirrors the `symbol_registry` pattern. - embedding_service: Arc>>, + /// Shared, per-model embedding-service pool — used by MCP sessions AND the + /// REST handlers so each ONNX embedding model is loaded ONCE per serve + /// instance (lazily, on the first semantic query) and reused across all + /// requests. Without this, per-request `CodesearchService` construction + /// (REST handlers) would reload the model on every call (~100ms–2s). + /// + /// A pool rather than a single service because serve is multi-repo and + /// indexes may be built with different models: every query must be embedded + /// with the model of the repo it targets. Mirrors the `symbol_registry` + /// pattern. + embedding_pool: Arc, + /// Serve-wide default embedding model for newly created indexes + /// (`codesearch serve --model `), or `None` for the built-in default. + /// + /// This never overrides an index that already records its own model, and it + /// is deliberately NOT the query fallback for an index that records none: + /// a legacy index with no `model_short_name` is queried with the built-in + /// default and reported with a warning (see + /// `CodesearchService::resolve_query_model`). Applying this flag there would + /// break a working legacy repo the moment an operator set it. The default + /// applies only when `POST /repos` creates a brand-new index without an + /// explicit `model`, and to the scope-free status summary. + default_model: Option, + /// Aliases for which the unrecorded-model query warning has already been + /// emitted, so a long-running serve logs it once per repo instead of once + /// per query. See [`Self::mark_legacy_model_warned`]. + legacy_model_warned: DashMap, /// Per-repo total tool call count. tool_call_counts: DashMap, /// Per-repo C# symbol index status (cached, updated on rebuild/detect). @@ -354,7 +375,11 @@ impl ServeState { total_sessions: AtomicU64::new(0), sysinfo_system: std::sync::Mutex::new(sys), symbol_registry: Arc::new(SymbolIndexerRegistry::new()), - embedding_service: Arc::new(std::sync::Mutex::new(None)), + embedding_pool: Arc::new(crate::embed::EmbeddingServicePool::new( + crate::constants::get_global_models_cache_dir().ok(), + )), + default_model: None, + legacy_model_warned: DashMap::new(), tool_call_counts: DashMap::new(), csharp_index_status: Arc::new(DashMap::new()), csharp_index_error: Arc::new(DashMap::new()), @@ -447,14 +472,56 @@ impl ServeState { Arc::clone(&self.symbol_registry) } - /// Return a clone of the shared embedding-service Arc. - /// Shared across MCP sessions AND REST handlers so the ONNX model is loaded + /// Return a clone of the shared, per-model embedding-service pool. + /// Shared across MCP sessions AND REST handlers so each ONNX model is loaded /// once per serve instance (lazily on first semantic query) instead of being /// reloaded per request/session. - pub(crate) fn embedding_service( - &self, - ) -> Arc>> { - Arc::clone(&self.embedding_service) + pub(crate) fn embedding_pool(&self) -> Arc { + Arc::clone(&self.embedding_pool) + } + + /// Attach the serve-wide default embedding model (`codesearch serve --model`). + /// + /// Set once at startup, before the state is shared. `None` leaves the + /// built-in default in place. + pub(crate) fn with_default_model(mut self, model: Option) -> Self { + self.default_model = model; + self + } + + /// The serve-wide default embedding model for newly created indexes, or + /// `None` for the built-in default. See [`Self::with_default_model`]. + pub(crate) fn default_model(&self) -> Option { + self.default_model + } + + /// Resolve the embedding model an alias's index was built with. + /// + /// Returns `None` when the alias is unknown or its index has no + /// `model_short_name` (unindexed / legacy), so callers can fall back to + /// [`crate::embed::ModelType::default`]. This is the read side of the + /// per-repo model contract: a query against `alias` MUST be embedded with + /// the model returned here, or the vector search fails with a dimension + /// mismatch (768-dim EmbeddingGemma index, 384-dim default query) or + /// silently compares incomparable vector spaces. + pub(crate) fn model_for_alias(&self, alias: &str) -> Option { + let cfg = self.config_snapshot(); + let project_path = cfg.resolve(alias)?; + crate::embed::ModelType::from_index_metadata(&project_path.join(DB_DIR_NAME)) + } + + /// Record that `alias` was queried with the built-in default because its + /// index records no embedding model, returning `true` on the first call for + /// that alias. + /// + /// An unrecorded model is unknowable, so the fallback warning is logged once + /// per repo per serve lifetime rather than on every query — a busy hub would + /// otherwise flood the log with the same line. The caller-facing response + /// warning is not deduped: an agent should see the assumption on each answer. + pub(crate) fn mark_legacy_model_warned(&self, alias: &str) -> bool { + self.legacy_model_warned + .insert(alias.to_string(), ()) + .is_none() } /// Return the instant when serve started, used to compute uptime. @@ -1995,7 +2062,7 @@ impl ServeState { let db_path = path.join(DB_DIR_NAME); // Open stores: existence check + write/readonly/conflicted logic. - let stores = match self.try_open_stores(alias, &db_path, false, force_readonly)? { + let stores = match self.try_open_stores(alias, &db_path, false, force_readonly, None)? { OpenedStores::Readonly(stores) => { // Already registered as Readonly by try_open_stores. // @@ -2197,7 +2264,7 @@ impl ServeState { let db_path = path.join(DB_DIR_NAME); // Open stores: existence check + write/readonly/conflicted logic. - let stores = match self.try_open_stores(alias, &db_path, false, force_readonly)? { + let stores = match self.try_open_stores(alias, &db_path, false, force_readonly, None)? { OpenedStores::Readonly(s) => { // Already registered as Readonly; touch and return. self.touch_access(alias); @@ -2443,12 +2510,20 @@ impl ServeState { /// /// `allow_create=false`: warmup / incremental reindex path — fails if DB is missing. /// `allow_create=true`: force-reindex / add-repo path — creates fresh DB if missing. + /// + /// `dimension_override` forces the embeddings dimension (e.g. a model + /// override on `POST /repos`); `None` reads it from `metadata.json`. The + /// caller must have made the on-disk store consistent with the override + /// (a fresh DB, or one whose data will be cleared by the reindex) — opening + /// a store at a different dimension than its vectors were written with + /// yields a dimension mismatch on the first insert. fn try_open_stores( &self, alias: &str, db_path: &Path, allow_create: bool, force_readonly: bool, + dimension_override: Option, ) -> std::result::Result { if !db_path.exists() && !allow_create { let parent = db_path @@ -2464,7 +2539,7 @@ impl ServeState { )); } - let dims = self.get_dimensions_for_path(db_path); + let dims = dimension_override.unwrap_or_else(|| self.get_dimensions_for_path(db_path)); // Read-only requested via the per-repo `repo_read_only` config flag: // open readonly directly and never attempt a write open. This makes @@ -3237,6 +3312,10 @@ async fn status_handler( let uptime_secs = state.started_at().elapsed().as_secs(); + // Serve-wide default model for newly created indexes (`serve --model`). + // `null` means the built-in default. + let default_model = state.default_model().map(|m| m.short_name()); + // CPU usage — reuse shared System instance so cpu_usage() can compute delta let cpu = { use sysinfo::ProcessesToUpdate; @@ -3247,6 +3326,7 @@ async fn status_handler( "version": env!("CARGO_PKG_VERSION"), "repos": repo_json, "active_sessions": active_sessions, + "default_model": default_model, "cpu_percent": "—", "uptime_secs": uptime_secs, })); @@ -3259,6 +3339,7 @@ async fn status_handler( "version": env!("CARGO_PKG_VERSION"), "repos": repo_json, "active_sessions": active_sessions, + "default_model": default_model, "cpu_percent": "—", "uptime_secs": uptime_secs, })); @@ -3291,6 +3372,7 @@ async fn status_handler( "version": env!("CARGO_PKG_VERSION"), "repos": repo_json, "active_sessions": active_sessions, + "default_model": default_model, "cpu_percent": cpu, "csharp_helper": csharp_helper, "ts_helper": ts_helper, @@ -3757,7 +3839,7 @@ async fn reindex_handler( // FSW not running -- open existing or create fresh DB. // allow_create=true so a force-reindex can recover a deleted DB. let cancel = CancellationToken::new(); - match state.try_open_stores(&alias, &db_path, true, false) { + match state.try_open_stores(&alias, &db_path, true, false, None) { Ok(OpenedStores::Write(s)) => { // Register as Write to block double-open races while we reindex. state.repos.insert( @@ -3969,6 +4051,29 @@ struct AddRepoRequest { model: Option, } +/// Decide the embedding model a `POST /repos` add should index with. +/// +/// Precedence: +/// 1. an explicit `model` in the request always wins (it forces a rebuild at +/// that model's dimension, which is the documented `index add --model` +/// behavior); +/// 2. otherwise the serve-wide default (`codesearch serve --model`) applies +/// **only when no model is recorded on disk** — i.e. this call is creating a +/// brand-new index; +/// 3. an index that already records its own model keeps it, exactly as if +/// `--model` had not been passed. +fn resolve_add_repo_model( + explicit: Option, + recorded: Option, + serve_default: Option, +) -> Option { + explicit.or(if recorded.is_none() { + serve_default + } else { + None + }) +} + /// Add-repo handler: POST /repos /// /// Registers a new repo in repos.json, opens the LMDB/Tantivy stores inline @@ -4010,6 +4115,42 @@ async fn add_repo_handler( ); } + // db_path is resolved before the model decision: whether a serve-wide + // default applies depends on whether the index already records a model. + let db_path = canonical_path.join(DB_DIR_NAME); + + // Parse the optional model override BEFORE opening the store: a fresh index + // must be created at the override's dimension, not the 384-dim default. + // Previously the store was opened at the default (or the previous metadata's) + // dimension and the override was only applied to metadata afterwards, so the + // reindex embedded 768-dim vectors into a 384-dim store and indexed nothing. + let explicit_model: Option = match body.model.as_deref() { + Some(model_str) => match crate::embed::ModelType::parse(model_str) { + Some(mt) => Some(mt), + None => { + return ( + StatusCode::BAD_REQUEST, + axum::response::Json(json!({ + "error": format!("Unknown model: '{}'. Use one of: {}", model_str, crate::embed::ModelType::valid_short_names()), + "status": "error" + })), + ); + } + }, + None => None, + }; + + // `codesearch serve --model X` sets the default for indexes created here. + // It applies only when no explicit `model` was given AND this POST is + // creating a brand-new index: an existing index keeps the model recorded in + // its `metadata.json`, exactly as if the flag had not been set. An explicit + // `model` still wins and rebuilds at that model's dimension. + let model_override = resolve_add_repo_model( + explicit_model, + crate::embed::ModelType::from_index_metadata(&db_path), + state.default_model(), + ); + // Register in repos.json let alias = { let mut config = match state.config.write() { @@ -4067,8 +4208,13 @@ async fn add_repo_handler( // This eliminates the LMDB double-open race that occurred when the old // path opened its own LMDB handle, conflicting with // calls from the serve's request handlers. - let db_path = canonical_path.join(DB_DIR_NAME); - let stores = match state.try_open_stores(&alias, &db_path, true, false) { + let stores = match state.try_open_stores( + &alias, + &db_path, + true, + false, + model_override.map(|m| m.dimensions()), + ) { Ok(OpenedStores::Write(s)) => s, Ok(OpenedStores::Readonly(_)) => { unreachable!( @@ -4139,23 +4285,6 @@ async fn add_repo_handler( ); } - // Parse optional model override from request body. - let model_override: Option = match body.model.as_deref() { - Some(model_str) => match crate::embed::ModelType::parse(model_str) { - Some(mt) => Some(mt), - None => { - return ( - StatusCode::BAD_REQUEST, - axum::response::Json(json!({ - "error": format!("Unknown model: '{}'. Use one of: {}", model_str, crate::embed::ModelType::valid_short_names()), - "status": "error" - })), - ); - } - }, - None => None, - }; - // Spawn the heavy indexing work in the background. Returns 202 immediately. let alias_bg = alias.clone(); let state_bg = state.clone(); @@ -4953,10 +5082,16 @@ fn keep_warm_foreign_target(ping_url: &str, self_host: &str) -> Option { } } +// `run_serve` is the single startup entry point, so its parameter list is the +// serve CLI surface (bind host/port, registration, default model, TUI, +// keep-warm, shutdown). Bundling them into a struct would only move the +// plumbing; allow the wide signature instead. +#[allow(clippy::too_many_arguments)] pub async fn run_serve( host: Option, port: Option, register_paths: Vec, + default_model: Option, no_tui: bool, keep_warm_url: Option, idle_suspend_secs: Option, @@ -5040,7 +5175,7 @@ pub async fn run_serve( // env > default); nothing else consumes it, so `ServeState` does not carry // it. In particular the embedded TUI must NOT derive a poll cadence from it // — it never polls a federated peer on a timer at all. - let serve_state = Arc::new(ServeState::new(config, None)); + let serve_state = Arc::new(ServeState::new(config, None).with_default_model(default_model)); // Construct the bind address from resolved host + port. // Using `format!` with `parse::()` handles both IPv4 and IPv6. @@ -5074,6 +5209,20 @@ pub async fn run_serve( info!("📋 Registered repos: {}", repo_list); eprintln!("📋 Registered repos: {}", repo_list); + // Report the serve-wide default model for newly created indexes, if set. + // Without this, `serve --model X` is a silent setting: the TUI/status show + // per-repo models, but nothing tells an operator what a new `POST /repos` + // (or a delegated `codesearch index add`) will use. + if let Some(model) = default_model { + let line = format!( + "🧠 Default model for new indexes: {} ({} dims)", + model.short_name(), + model.dimensions() + ); + info!("{}", line); + eprintln!("{}", line); + } + // ── Start HTTP server FIRST ── // Accept connections immediately so MCP clients don't time out. // Pre-warming runs in the background below. diff --git a/src/serve/tests.rs b/src/serve/tests.rs index 8518a033..d663e91b 100644 --- a/src/serve/tests.rs +++ b/src/serve/tests.rs @@ -770,7 +770,7 @@ async fn try_open_stores_creates_db_for_brand_new_repo() { let state = state_with_config(ReposConfig::default()); - match state.try_open_stores("brandnew", &db_path, true, false) { + match state.try_open_stores("brandnew", &db_path, true, false, None) { Ok(OpenedStores::Write(_)) => {} Ok(OpenedStores::Readonly(_)) => { panic!("brand-new repo opened Readonly; expected Write") @@ -851,6 +851,250 @@ async fn add_repo_handler_registers_brand_new_repo_without_rollback() { ); } +/// `POST /repos` with no explicit `model` must create a brand-new index at the +/// serve-wide default's dimension (`codesearch serve --model X`), not the +/// built-in 384-dim default. This is the write-side counterpart of the per-repo +/// query-model contract. +#[tokio::test] +async fn add_repo_handler_uses_serve_default_model_for_new_index() { + let tmp = tempfile::tempdir().unwrap(); + let repo_path = tmp.path().join("defaulted"); + std::fs::create_dir(&repo_path).unwrap(); + + let state = Arc::new( + state_with_config(ReposConfig::default()) + .with_default_model(Some(crate::embed::ModelType::EmbeddingGemma300MQ4)), + ); + + let (status, body) = add_repo_handler( + axum::extract::State(state.clone()), + axum::extract::Json(AddRepoRequest { + path: repo_path.clone(), + alias: Some("defaulted".to_string()), + model: None, + }), + ) + .await; + assert_eq!( + status, + axum::http::StatusCode::ACCEPTED, + "add must be accepted, got {}: {}", + status, + body.0 + ); + + let stores = state + .get_opened_stores("defaulted") + .expect("store must be open immediately after add"); + let dims = stores + .vector_store + .try_read() + .unwrap() + .stats() + .unwrap() + .dimensions; + assert_eq!( + dims, + crate::embed::ModelType::EmbeddingGemma300MQ4.dimensions(), + "a new index must be created at the serve default model's dimension" + ); +} + +/// The serve-wide default must NOT override an index that already records its +/// own model: re-adding a repo whose `.codesearch.db` is still on disk keeps the +/// recorded model and dimension. +#[tokio::test] +async fn add_repo_handler_keeps_recorded_model_over_serve_default() { + let tmp = tempfile::tempdir().unwrap(); + let repo_path = tmp.path().join("existing"); + std::fs::create_dir(&repo_path).unwrap(); + let db_path = repo_path.join(DB_DIR_NAME); + + // A pre-existing index recording the 384-dim default model. + std::fs::create_dir_all(&db_path).unwrap(); + let mut meta = serde_json::Map::new(); + crate::embed::ModelType::AllMiniLML6V2Q.write_metadata_fields(&mut meta); + std::fs::write( + db_path.join("metadata.json"), + serde_json::to_string(&meta).unwrap(), + ) + .unwrap(); + + let state = Arc::new( + state_with_config(ReposConfig::default()) + .with_default_model(Some(crate::embed::ModelType::EmbeddingGemma300MQ4)), + ); + + let (status, body) = add_repo_handler( + axum::extract::State(state.clone()), + axum::extract::Json(AddRepoRequest { + path: repo_path.clone(), + alias: Some("existing".to_string()), + model: None, + }), + ) + .await; + assert_eq!( + status, + axum::http::StatusCode::ACCEPTED, + "add must be accepted, got {}: {}", + status, + body.0 + ); + + let stores = state + .get_opened_stores("existing") + .expect("store must be open immediately after add"); + let dims = stores + .vector_store + .try_read() + .unwrap() + .stats() + .unwrap() + .dimensions; + assert_eq!( + dims, + crate::embed::ModelType::AllMiniLML6V2Q.dimensions(), + "an existing index must keep its recorded model, not adopt the serve default" + ); +} + +/// Precedence contract for the model a `POST /repos` add indexes with. +#[test] +fn resolve_add_repo_model_precedence() { + use crate::embed::ModelType; + let gemma = ModelType::EmbeddingGemma300MQ4; + let mini = ModelType::AllMiniLML6V2Q; + + // Explicit model always wins, even over a recorded model and a default. + assert_eq!( + resolve_add_repo_model(Some(gemma), Some(mini), Some(mini)), + Some(gemma) + ); + // No explicit model, no recorded model → serve default applies (new index). + assert_eq!(resolve_add_repo_model(None, None, Some(gemma)), Some(gemma)); + // No explicit model, recorded model present → serve default is ignored. + assert_eq!(resolve_add_repo_model(None, Some(mini), Some(gemma)), None); + // No explicit model, no recorded model, no default → no override. + assert_eq!(resolve_add_repo_model(None, None, None), None); + // Explicit model still wins when nothing else is set. + assert_eq!(resolve_add_repo_model(Some(mini), None, None), Some(mini)); +} + +/// The serve-wide default is the scope-free fallback model in serve mode (the +/// unpinned status summary, a call with no routed alias). It is deliberately +/// NOT the query fallback for a repo that records no model — see +/// `unrecorded_index_is_queried_with_builtin_default_not_serve_default`. +#[test] +fn serve_default_model_is_service_fallback() { + use crate::embed::ModelType; + let state = std::sync::Arc::new( + ServeState::new(ReposConfig::default(), None) + .with_default_model(Some(ModelType::EmbeddingGemma300MQ4)), + ); + assert_eq!(state.default_model(), Some(ModelType::EmbeddingGemma300MQ4)); + + let svc = crate::mcp::CodesearchService::new_for_serve(state).unwrap(); + assert_eq!( + svc.query_model(None), + ModelType::EmbeddingGemma300MQ4, + "serve must fall back to its default model, not the built-in default" + ); +} + +/// Without `--model`, `ServeState` reports no default and the service falls back +/// to the built-in default. +#[test] +fn no_serve_default_keeps_builtin_fallback() { + use crate::embed::ModelType; + let state = std::sync::Arc::new(ServeState::new(ReposConfig::default(), None)); + assert_eq!(state.default_model(), None); + + let svc = crate::mcp::CodesearchService::new_for_serve(state).unwrap(); + assert_eq!(svc.query_model(None), ModelType::default()); +} + +/// A repo whose `metadata.json` records no model is queried with the BUILT-IN +/// default, never the serve-wide `--model` default. +/// +/// Regression guard for `serve --model X` silently overriding a legacy index: +/// with a 768-dim serve default, a 384-dim legacy index failed every search with +/// "Query embedding dimension mismatch: expected 384, got 768", and a +/// same-dimension default would have compared incomparable vector spaces without +/// erroring. The assumption must also reach the caller as a warning naming the +/// repo, the assumed model and the re-index command. +#[test] +fn unrecorded_index_is_queried_with_builtin_default_not_serve_default() { + use crate::embed::ModelType; + + let tmp = tempfile::tempdir().unwrap(); + let config_file = tmp.path().join("repos.json"); + let repo_path = tmp.path().join("legacy"); + std::fs::create_dir(&repo_path).unwrap(); + let mut config = ReposConfig::default(); + config + .register_with_alias(repo_path.clone(), Some("legacy".to_string())) + .unwrap(); + config.save_to(&config_file).unwrap(); + + let state = std::sync::Arc::new( + ServeState::new(config, Some(config_file)) + .with_default_model(Some(ModelType::EmbeddingGemma300MQ4)), + ); + let svc = crate::mcp::CodesearchService::new_for_serve(state).unwrap(); + + // No metadata.json yet: an unrecorded model. Serve default is gemma. + let resolution = svc.resolve_query_model(Some("legacy")); + assert_eq!( + resolution.model, + ModelType::default(), + "a repo that records no model must be queried with the built-in default, \ + not the '{}' serve default", + ModelType::EmbeddingGemma300MQ4.short_name() + ); + let warning = resolution + .assumed_warning + .expect("the assumed model must be surfaced to the caller"); + assert!( + warning.contains("legacy"), + "warning must name the repo: {warning}" + ); + assert!( + warning.contains(ModelType::default().short_name()), + "warning must name the assumed model: {warning}" + ); + assert!( + warning.contains("--force"), + "warning must give the re-index command: {warning}" + ); + + // A recorded model is used as-is and must not warn. + let db_path = repo_path.join(DB_DIR_NAME); + std::fs::create_dir_all(&db_path).unwrap(); + std::fs::write( + db_path.join("metadata.json"), + r#"{"model_short_name":"embeddinggemma-q4","dimensions":768}"#, + ) + .unwrap(); + let resolution = svc.resolve_query_model(Some("legacy")); + assert_eq!(resolution.model, ModelType::EmbeddingGemma300MQ4); + assert!( + resolution.assumed_warning.is_none(), + "a recorded model must not warn" + ); +} + +/// The unrecorded-model log warning fires once per alias, so a busy hub does not +/// repeat the same line on every query. The caller-facing warning is separate +/// and is not deduped. +#[test] +fn legacy_model_warning_is_logged_once_per_alias() { + let state = ServeState::new(ReposConfig::default(), None); + assert!(state.mark_legacy_model_warned("a")); + assert!(!state.mark_legacy_model_warned("a")); + assert!(state.mark_legacy_model_warned("b")); +} + /// `persist_config` must write to the override path (and therefore be /// observable by `reload_if_changed`/`config_snapshot`) rather than the real /// `~/.codesearch/repos.json`. Guards the wiring that makes the register @@ -2306,7 +2550,7 @@ async fn index_rm_deletes_db_while_serve_holds_real_lmdb_env() { // Serve opens the repo FOR REAL — a live LMDB env under db_path. let opened = state - .try_open_stores("heldenv", &db_path, true, false) + .try_open_stores("heldenv", &db_path, true, false, None) .expect("opening a real store for a brand-new repo must succeed"); let OpenedStores::Write(stores) = opened else { panic!("brand-new repo must open Write, not Readonly"); @@ -2404,3 +2648,190 @@ fn evicting_idle_repo_clears_frozen_csharp_error_state() { "eviction must clear the cached C# error message along with the status" ); } + +/// The model a serve query is embedded with is read from the routed repo's own +/// index metadata — never assumed to be the hub-wide default. +/// +/// Regression guard for the serve hub pinning `ModelType::default()` (384-dim +/// MiniLM) for every query: on a hub whose indexes were rebuilt with +/// EmbeddingGemma that failed with "Query embedding dimension mismatch: +/// expected 768, got 384". Reintroducing the default pin makes the gemma cases +/// below fail. +#[test] +fn model_for_alias_reads_the_index_metadata_model() { + let cases = [ + ( + "embeddinggemma-q4", + Some(crate::embed::ModelType::EmbeddingGemma300MQ4), + ), + ("minilm-l6-q", Some(crate::embed::ModelType::AllMiniLML6V2Q)), + ("bge-base", Some(crate::embed::ModelType::BGEBaseENV15)), + // An unknown recorded name must not be silently coerced to the default: + // callers fall back explicitly, and the resolver reports "no answer". + ("not-a-real-model", None), + ]; + + for (model_short_name, expected) in cases { + let (_tmp, repo_path, state) = state_with_repo("repo"); + let db_path = repo_path.join(DB_DIR_NAME); + std::fs::create_dir_all(&db_path).unwrap(); + std::fs::write( + db_path.join("metadata.json"), + format!(r#"{{"model_short_name":"{model_short_name}","dimensions":768}}"#), + ) + .unwrap(); + + assert_eq!( + state.model_for_alias("repo"), + expected, + "metadata model_short_name '{model_short_name}' must drive the query model" + ); + } +} + +/// Missing metadata (unindexed / legacy index) yields `None`, so the caller's +/// documented fallback to the default applies — and an unknown alias cannot +/// borrow another repo's model. +#[test] +fn model_for_alias_is_none_without_index_metadata() { + let (_tmp, _repo_path, state) = state_with_repo("repo"); + assert_eq!(state.model_for_alias("repo"), None); + assert_eq!(state.model_for_alias("not-registered"), None); +} + +/// A single hub can hold indexes built with different models: each alias +/// resolves independently, so a group fan-out embeds each store's query with +/// that store's own model. +#[test] +fn model_for_alias_is_per_repo_not_hub_wide() { + let tmp = tempfile::tempdir().unwrap(); + let config_file = tmp.path().join("repos.json"); + let mut config = ReposConfig::default(); + for (alias, model) in [("legacy", "minilm-l6-q"), ("rebuilt", "embeddinggemma-q4")] { + let repo_path = tmp.path().join(alias); + std::fs::create_dir(&repo_path).unwrap(); + config + .register_with_alias(repo_path.clone(), Some(alias.to_string())) + .unwrap(); + let db_path = repo_path.join(DB_DIR_NAME); + std::fs::create_dir_all(&db_path).unwrap(); + std::fs::write( + db_path.join("metadata.json"), + format!(r#"{{"model_short_name":"{model}"}}"#), + ) + .unwrap(); + } + config.save_to(&config_file).unwrap(); + let state = ServeState::new(config, Some(config_file)); + + assert_eq!( + state.model_for_alias("legacy"), + Some(crate::embed::ModelType::AllMiniLML6V2Q) + ); + assert_eq!( + state.model_for_alias("rebuilt"), + Some(crate::embed::ModelType::EmbeddingGemma300MQ4) + ); +} + +/// The serve MCP service resolves the query model through the routed repo, not +/// its own (default) field. This is the exact seam the hub got wrong: it is the +/// service, not `ServeState`, that hands the model to the embedder. +#[test] +fn serve_service_uses_repo_model_not_default() { + let (_tmp, repo_path, state) = state_with_repo("gemma-repo"); + let db_path = repo_path.join(DB_DIR_NAME); + std::fs::create_dir_all(&db_path).unwrap(); + std::fs::write( + db_path.join("metadata.json"), + r#"{"model_short_name":"embeddinggemma-q4","dimensions":768}"#, + ) + .unwrap(); + + let svc = crate::mcp::CodesearchService::new_for_serve(std::sync::Arc::new(state)).unwrap(); + + assert_eq!( + svc.query_model(Some("gemma-repo")), + crate::embed::ModelType::EmbeddingGemma300MQ4, + "serve must embed a repo's queries with the model that repo was indexed with" + ); + // No alias (unscoped) or an unknown alias falls back to the service default. + assert_eq!(svc.query_model(None), crate::embed::ModelType::default()); + assert_eq!( + svc.query_model(Some("not-registered")), + crate::embed::ModelType::default() + ); +} + +/// The grouped `status` model label must reflect the members' recorded models, +/// not the service default: a same-model group names that model, a mixed-model +/// hub says `mixed`. Regression guard for the status field reporting the +/// hardcoded default (`minilm-l6-q`) for every repo. +#[test] +fn group_status_model_label_is_common_or_mixed() { + let tmp = tempfile::tempdir().unwrap(); + let config_file = tmp.path().join("repos.json"); + let mut config = ReposConfig::default(); + for (alias, model) in [("legacy", "minilm-l6-q"), ("rebuilt", "embeddinggemma-q4")] { + let repo_path = tmp.path().join(alias); + std::fs::create_dir(&repo_path).unwrap(); + config + .register_with_alias(repo_path.clone(), Some(alias.to_string())) + .unwrap(); + let db_path = repo_path.join(DB_DIR_NAME); + std::fs::create_dir_all(&db_path).unwrap(); + std::fs::write( + db_path.join("metadata.json"), + format!(r#"{{"model_short_name":"{model}"}}"#), + ) + .unwrap(); + } + config.save_to(&config_file).unwrap(); + let state = std::sync::Arc::new(ServeState::new(config, Some(config_file))); + let svc = crate::mcp::CodesearchService::new_for_serve(state).unwrap(); + + assert_eq!( + svc.group_model_label(&["legacy".to_string(), "rebuilt".to_string()]), + "mixed", + "a hub holding indexes built with different models must report 'mixed'" + ); + assert_eq!( + svc.group_model_label(&["rebuilt".to_string()]), + "embeddinggemma-q4", + "a single-model group must name its base model" + ); +} + +/// A fresh repo added with a model override must open its store at that model's +/// dimension, not the 384-dim default. Regression guard for `POST /repos` with +/// `model=embeddinggemma-q4`: the store used to be created at 384 and the +/// override applied only to metadata, so the reindex embedded 768-dim vectors +/// into a 384-dim store and indexed nothing. +#[tokio::test] +async fn try_open_stores_honours_dimension_override_for_a_fresh_repo() { + let tmp = tempfile::tempdir().unwrap(); + let repo_path = tmp.path().join("gemmarepo"); + std::fs::create_dir(&repo_path).unwrap(); + let db_path = repo_path.join(DB_DIR_NAME); + assert!(!db_path.exists(), "precondition: db dir must not exist yet"); + + let state = state_with_config(ReposConfig::default()); + + let stores = match state.try_open_stores("gemmarepo", &db_path, true, false, Some(768)) { + Ok(OpenedStores::Write(s)) => s, + Ok(OpenedStores::Readonly(_)) => panic!("expected Write, got Readonly"), + Err(e) => panic!("fresh open with a dimension override must succeed, got: {e}"), + }; + + let dims = stores + .vector_store + .read() + .await + .stats() + .expect("stats on a freshly created store") + .dimensions; + assert_eq!( + dims, 768, + "a repo added with --model embeddinggemma-q4 must open at 768 dims, not the 384 default" + ); +} diff --git a/src/serve/tui.rs b/src/serve/tui.rs index de7ea5d8..b82834f5 100644 --- a/src/serve/tui.rs +++ b/src/serve/tui.rs @@ -1120,7 +1120,7 @@ fn spawn_force_reindex(alias: String, state: &Arc) -> ReindexLaunch None => { // Try to open stores (allow_create=true for recovery) let cancel = CancellationToken::new(); - match state.try_open_stores(&alias, &db_path, true, false) { + match state.try_open_stores(&alias, &db_path, true, false, None) { Ok(super::OpenedStores::Write(s)) => { state.repos.insert( alias.clone(),