From 6e472c57b70f113c41c82cf78d0043600c91de6c Mon Sep 17 00:00:00 2001 From: kewton Date: Wed, 25 Mar 2026 20:05:00 +0900 Subject: [PATCH] fix(embedding): change default embedding model to bge-m3 - Change default model from nomic-embed-text to qllama/bge-m3:q8_0 for significantly better multilingual (especially Japanese) search - DRY: resolve_config() now calls default_model() instead of hardcoding - Add model_not_found_hint() shared helper for install guidance - Move delete_stale_model_embeddings() after first successful embed to prevent data loss when new model is not installed - Add ModelNotFound early exit in embed/index commands - Update README with new default, migration guide - Update test assertions and add new tests for hint, dimension, default Closes #177 Co-Authored-By: Claude Opus 4.6 (1M context) --- README.md | 20 ++++++++++----- src/cli/embed.rs | 27 ++++++++++++++++----- src/cli/index.rs | 22 ++++++++++++----- src/config/mod.rs | 6 ++--- src/embedding/mod.rs | 47 +++++++++++++++++++++++++++++++++--- tests/e2e_semantic_hybrid.rs | 7 ++++++ 6 files changed, 104 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index 66cddb4..4fa1ed0 100644 --- a/README.md +++ b/README.md @@ -53,8 +53,8 @@ commandindex diff src/auth/jwt.rs src/auth/middleware.rs --format json | モデル | 次元数 | 特徴 | |---|---|---| -| `nomic-embed-text` | 768 | デフォルト。英語中心 | -| `qllama/bge-m3:q8_0` | 1024 | 多言語対応（日本語に強い） | +| `qllama/bge-m3:q8_0` | 1024 | デフォルト。多言語対応（日本語に強い） | +| `nomic-embed-text` | 768 | 英語中心 | ### 前提条件 @@ -63,10 +63,10 @@ commandindex diff src/auth/jwt.rs src/auth/middleware.rs --format json ```bash # デフォルトモデル -ollama pull nomic-embed-text - -# 日本語対応モデル ollama pull qllama/bge-m3:q8_0 + +# 英語中心モデル +ollama pull nomic-embed-text ``` ### モデル変更手順 @@ -75,11 +75,19 @@ ollama pull qllama/bge-m3:q8_0 ```toml [embedding] -model = "qllama/bge-m3:q8_0" +model = "nomic-embed-text" ``` > **注意:** モデル変更後の再生成にはファイル数に応じた時間がかかります。 +### v0.x.x からの移行 + +v0.x.x 以前からアップグレードした場合、デフォルトモデルが `nomic-embed-text` から `qllama/bge-m3:q8_0` に変更されています。 + +1. 新しいデフォルトモデルをインストール: `ollama pull qllama/bge-m3:q8_0` +2. `commandindex embed` または `commandindex index --with-embedding` を実行すると、旧モデルの embedding は自動的に削除され、新モデルで再生成されます。 +3. 旧モデルを引き続き使用する場合は、`commandindex.toml` に `[embedding]` セクションで `model = "nomic-embed-text"` を指定してください。 + ## 開発 ### 前提条件 diff --git a/src/cli/embed.rs b/src/cli/embed.rs index 296a7c8..d388951 100644 --- a/src/cli/embed.rs +++ b/src/cli/embed.rs @@ -12,7 +12,7 @@ use std::time::{Duration, Instant}; use crate::config::{ConfigError, load_config}; use crate::embedding::store::{EmbeddingStore, EmbeddingStoreError}; -use crate::embedding::{EmbeddingError, create_provider}; +use crate::embedding::{EmbeddingError, create_provider, model_not_found_hint}; use crate::indexer::manifest::{Manifest, ManifestError}; use crate::indexer::reader::{IndexReaderWrapper, ReaderError}; @@ -141,17 +141,13 @@ pub fn run(path: &Path, commandindex_dir: &Path) -> Result 0 { - eprintln!("Info: Deleted {stale_deleted} stale embeddings from previous model."); - } let mut total_sections: u64 = 0; let mut generated: u64 = 0; let mut cached: u64 = 0; let mut failed: u64 = 0; + let mut stale_deleted = false; // 7. Process each file entry for entry in &manifest.files { @@ -185,6 +181,15 @@ pub fn run(path: &Path, commandindex_dir: &Path) -> Result { + // Delete stale embeddings once after first successful embed + if !stale_deleted { + let count = store.delete_stale_model_embeddings(model_name)?; + if count > 0 { + eprintln!("Info: Deleted {count} stale embeddings from previous model."); + } + stale_deleted = true; + } + if sections.len() != embeddings.len() { eprintln!( "Warning: section/embedding count mismatch for {}: {} sections, {} embeddings", @@ -221,6 +226,16 @@ pub fn run(path: &Path, commandindex_dir: &Path) -> Result { + eprintln!("{}", model_not_found_hint(&model)); + return Ok(EmbedSummary { + total_sections, + generated, + cached, + failed, + duration: start.elapsed(), + }); + } Err(e) => { eprintln!( "Warning: embedding generation failed for {}: {e}", diff --git a/src/cli/index.rs b/src/cli/index.rs index 66d1f06..8f94d14 100644 --- a/src/cli/index.rs +++ b/src/cli/index.rs @@ -25,7 +25,7 @@ use chrono::{DateTime, Utc}; use crate::config::{ConfigError, load_config}; use crate::embedding::store::{EmbeddingStore, EmbeddingStoreError}; -use crate::embedding::{EmbeddingError, create_provider}; +use crate::embedding::{EmbeddingError, create_provider, model_not_found_hint}; use crate::indexer::diff::{DiffError, detect_changes, scan_files}; use crate::indexer::manifest::{ self, FileEntry, FileType, Manifest, ManifestError, to_relative_path_string, @@ -929,18 +929,15 @@ fn generate_embeddings_for_manifest( let store = EmbeddingStore::open(&db_path)?; store.create_tables()?; - // Delete stale embeddings from previous model let model_name = provider.model_name(); - let stale_deleted = store.delete_stale_model_embeddings(model_name)?; - if stale_deleted > 0 { - eprintln!("Info: Deleted {stale_deleted} stale embeddings from previous model."); - } let tantivy_dir = crate::indexer::index_dir(commandindex_dir); let reader = IndexReaderWrapper::open(&tantivy_dir).map_err(|e| { IndexError::IndexCorrupted(format!("Failed to open tantivy for embedding: {e}")) })?; + let mut stale_deleted = false; + for entry in &manifest.files { if store.has_current_embedding(&entry.path, &entry.hash, model_name)? { continue; @@ -966,6 +963,15 @@ fn generate_embeddings_for_manifest( match provider.embed(&texts) { Ok(embeddings) => { + // Delete stale embeddings once after first successful embed + if !stale_deleted { + let count = store.delete_stale_model_embeddings(model_name)?; + if count > 0 { + eprintln!("Info: Deleted {count} stale embeddings from previous model."); + } + stale_deleted = true; + } + let dimension = provider.dimension(); let model = provider.model_name(); if sections.len() != embeddings.len() { @@ -995,6 +1001,10 @@ fn generate_embeddings_for_manifest( ); } } + Err(EmbeddingError::ModelNotFound(model)) => { + eprintln!("{}", model_not_found_hint(&model)); + return Ok(()); + } Err(e) => { eprintln!( "Warning: embedding generation failed for {}: {e}", diff --git a/src/config/mod.rs b/src/config/mod.rs index 7a74a51..1f25216 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -458,7 +458,7 @@ fn resolve_config(raw: RawConfig, sources: Vec) -> AppConfig { let embedding = if let Some(emb) = raw.embedding { EmbeddingConfig { provider: emb.provider.unwrap_or_default(), - model: emb.model.unwrap_or_else(|| "nomic-embed-text".to_string()), + model: emb.model.unwrap_or_else(crate::embedding::default_model), endpoint: emb .endpoint .unwrap_or_else(|| "http://localhost:11434".to_string()), @@ -687,7 +687,7 @@ mod tests { assert_eq!(config.search.snippet_lines, 2); assert_eq!(config.search.snippet_chars, 120); assert_eq!(config.embedding.provider, ProviderType::Ollama); - assert_eq!(config.embedding.model, "nomic-embed-text"); + assert_eq!(config.embedding.model, "qllama/bge-m3:q8_0"); assert_eq!(config.embedding.endpoint, "http://localhost:11434"); assert!(config.embedding.api_key.is_none()); assert_eq!(config.rerank.model, "llama3"); @@ -1015,7 +1015,7 @@ timeout_secs = 60 }, embedding: EmbeddingConfig { provider: ProviderType::Ollama, - model: "nomic-embed-text".to_string(), + model: "qllama/bge-m3:q8_0".to_string(), endpoint: "http://localhost:11434".to_string(), api_key: Some("secret".to_string()), }, diff --git a/src/embedding/mod.rs b/src/embedding/mod.rs index 1a1c309..b2b1f64 100644 --- a/src/embedding/mod.rs +++ b/src/embedding/mod.rs @@ -79,8 +79,8 @@ pub enum ProviderType { // EmbeddingConfig // --------------------------------------------------------------------------- -fn default_model() -> String { - "nomic-embed-text".to_string() +pub(crate) fn default_model() -> String { + "qllama/bge-m3:q8_0".to_string() } fn default_endpoint() -> String { @@ -133,6 +133,13 @@ impl EmbeddingConfig { // Shared utilities for providers // --------------------------------------------------------------------------- +/// ModelNotFound時のインストール案内メッセージを返す。 +pub(crate) fn model_not_found_hint(model: &str) -> String { + format!( + "Model '{model}' not found. Install it with:\n ollama pull {model}\nThen retry the command." + ) +} + /// HTTPレスポンスのステータスコードをEmbeddingErrorに変換する。 /// OllamaProvider・OpenAiProvider共通のエラーハンドリング。 pub(crate) fn map_status_to_error( @@ -246,7 +253,7 @@ mod tests { fn test_embedding_config_default() { let config = EmbeddingConfig::default(); assert_eq!(config.provider, ProviderType::Ollama); - assert_eq!(config.model, "nomic-embed-text"); + assert_eq!(config.model, "qllama/bge-m3:q8_0"); assert_eq!(config.endpoint, "http://localhost:11434"); assert!(config.api_key.is_none()); } @@ -295,7 +302,7 @@ provider = "ollama" "#; let emb: EmbeddingConfig = toml::from_str(toml_str).unwrap(); assert_eq!(emb.provider, ProviderType::Ollama); - assert_eq!(emb.model, "nomic-embed-text"); + assert_eq!(emb.model, "qllama/bge-m3:q8_0"); assert_eq!(emb.endpoint, "http://localhost:11434"); assert!(emb.api_key.is_none()); } @@ -419,4 +426,36 @@ provider = "ollama" let provider = create_provider(&config).unwrap(); assert_eq!(provider.provider_name(), "openai"); } + + // --- model_not_found_hint --- + + #[test] + fn test_model_not_found_hint_contains_model_name() { + let hint = model_not_found_hint("qllama/bge-m3:q8_0"); + assert!(hint.contains("qllama/bge-m3:q8_0")); + assert!(hint.contains("ollama pull")); + } + + #[test] + fn test_model_not_found_hint_contains_retry_instruction() { + let hint = model_not_found_hint("some-model"); + assert!(hint.contains("some-model")); + assert!(hint.contains("retry")); + } + + // --- default_model returns bge-m3 --- + + #[test] + fn test_default_model_is_bge_m3() { + assert_eq!(default_model(), "qllama/bge-m3:q8_0"); + } + + // --- default config dimension is 1024 (bge-m3) --- + + #[test] + fn test_default_config_dimension_is_1024() { + let config = EmbeddingConfig::default(); + let provider = create_provider(&config).unwrap(); + assert_eq!(provider.dimension(), 1024); + } } diff --git a/tests/e2e_semantic_hybrid.rs b/tests/e2e_semantic_hybrid.rs index 8483c51..e8ba91d 100644 --- a/tests/e2e_semantic_hybrid.rs +++ b/tests/e2e_semantic_hybrid.rs @@ -337,6 +337,13 @@ fn test_embed_without_ollama_fails() { let (dir, _commandindex_dir) = setup_semantic_test_dir().expect("test_embed_without_ollama_fails: setup"); + // Force embed to fail by pointing to unreachable endpoint + fs::write( + dir.path().join("commandindex.toml"), + "[embedding]\nendpoint = \"http://127.0.0.1:19999\"\n", + ) + .expect("test_embed_without_ollama_fails: write config"); + // Running embed without Ollama available exits successfully but reports // failures in stderr warnings and "Failed: N" in stdout. let output = common::cmd()