diff --git a/app/src/components/settings/panels/EmbeddingsPanel.tsx b/app/src/components/settings/panels/EmbeddingsPanel.tsx index 8485ffb5b..b2849af2a 100644 --- a/app/src/components/settings/panels/EmbeddingsPanel.tsx +++ b/app/src/components/settings/panels/EmbeddingsPanel.tsx @@ -321,10 +321,20 @@ const EmbeddingsPanel = ({ embedded = false }: EmbeddingsPanelProps = {}) => { result.error === 'EMBEDDINGS_NO_MODEL_LOADED' || result.error === 'EMBEDDINGS_VERIFICATION_FAILED' ) { - setSetupError( + // `result.message`/`result.detail` are backend-emitted (already + // context-specific); only the generic fallback is frontend-owned UI + // text, so route just that through useT() (#4056 CodeRabbit). + const baseMessage = typeof result.message === 'string' ? result.message - : "Couldn't verify the embeddings endpoint. Make sure it's running and serving an embedding model, then save again." + : t('settings.embeddings.verifyFallback'); + // Append the underlying probe failure (HTTP status / server error body) + // so the user can self-diagnose instead of seeing only the generic + // message (#4056). + setSetupError( + typeof result.detail === 'string' && result.detail.trim() + ? `${baseMessage} (${result.detail})` + : baseMessage ); setStatus({ kind: 'idle' }); return; diff --git a/app/src/components/settings/panels/__tests__/EmbeddingsPanel.test.tsx b/app/src/components/settings/panels/__tests__/EmbeddingsPanel.test.tsx index b1380447e..265183aa8 100644 --- a/app/src/components/settings/panels/__tests__/EmbeddingsPanel.test.tsx +++ b/app/src/components/settings/panels/__tests__/EmbeddingsPanel.test.tsx @@ -522,6 +522,37 @@ describe('EmbeddingsPanel', () => { expect(screen.getByPlaceholderText(/https:\/\/your-endpoint/i)).toBeInTheDocument(); }); + it('appends the underlying probe detail to the verification error so the user can self-diagnose (#4056)', async () => { + // The issue asks for the underlying HTTP status / error body, not just the + // generic message. When the backend supplies `detail`, the panel appends it. + const settings = makeSettings({ + providers: [ + makeProvider('managed', { requires_api_key: false }), + makeProvider('custom', { requires_api_key: false, requires_endpoint: true }), + ], + }); + vi.mocked(loadEmbeddingsSettings).mockResolvedValue(settings); + vi.mocked(updateEmbeddingsSettings).mockResolvedValue({ + error: 'EMBEDDINGS_VERIFICATION_FAILED', + message: "Couldn't verify the embeddings endpoint — the test embed failed.", + detail: 'Embedding API error (401 Unauthorized): invalid api key', + }); + + renderWithProviders(); + await screen.findByText('Custom'); + + fireEvent.click(screen.getByRole('radio', { name: /custom/i })); + await screen.findByPlaceholderText(/https:\/\/your-endpoint/i); + fireEvent.change(screen.getByPlaceholderText(/https:\/\/your-endpoint/i), { + target: { value: 'https://api.example.com/v1' }, + }); + fireEvent.click(screen.getByRole('button', { name: /save.*switch/i })); + + // The detail (HTTP status + body) is shown alongside the generic message. + await screen.findByText(/401 Unauthorized/i); + expect(screen.getByPlaceholderText(/https:\/\/your-endpoint/i)).toBeInTheDocument(); + }); + // ─── Confirm wipe dialog ────────────────────────────────────────────────── it('shows confirm-wipe dialog when updateEmbeddingsSettings returns DIMENSION_CHANGE error', async () => { diff --git a/app/src/lib/i18n/ar.ts b/app/src/lib/i18n/ar.ts index 1b9eae62b..5560c9ecb 100644 --- a/app/src/lib/i18n/ar.ts +++ b/app/src/lib/i18n/ar.ts @@ -1264,6 +1264,8 @@ const messages: TranslationMap = { 'settings.embeddings.testing': 'جارٍ الاختبار…', 'settings.embeddings.testSuccess': 'متصل — {dims} بُعد', 'settings.embeddings.connectionTestFailed': 'فشل الاختبار', + 'settings.embeddings.verifyFallback': + 'تعذّر التحقق من نقطة نهاية التضمين. تأكد من أنها قيد التشغيل وتوفّر نموذج تضمين، ثم احفظ مرة أخرى.', 'settings.embeddings.testFailed': 'فشل: {error}', 'settings.embeddings.saving': 'جارٍ الحفظ…', 'settings.embeddings.saved': 'تم الحفظ.', diff --git a/app/src/lib/i18n/bn.ts b/app/src/lib/i18n/bn.ts index 178d893b3..f24cf480f 100644 --- a/app/src/lib/i18n/bn.ts +++ b/app/src/lib/i18n/bn.ts @@ -1285,6 +1285,8 @@ const messages: TranslationMap = { 'settings.embeddings.testing': 'পরীক্ষা হচ্ছে…', 'settings.embeddings.testSuccess': 'সংযুক্ত — {dims} মাত্রা', 'settings.embeddings.connectionTestFailed': 'পরীক্ষা ব্যর্থ হয়েছে', + 'settings.embeddings.verifyFallback': + 'এমবেডিংস এন্ডপয়েন্ট যাচাই করা যায়নি। নিশ্চিত করুন এটি চলছে এবং একটি এমবেডিং মডেল পরিবেশন করছে, তারপর আবার সংরক্ষণ করুন।', 'settings.embeddings.testFailed': 'ব্যর্থ: {error}', 'settings.embeddings.saving': 'সংরক্ষণ হচ্ছে…', 'settings.embeddings.saved': 'সংরক্ষিত।', diff --git a/app/src/lib/i18n/de.ts b/app/src/lib/i18n/de.ts index 79b108b86..e79e704dd 100644 --- a/app/src/lib/i18n/de.ts +++ b/app/src/lib/i18n/de.ts @@ -1320,6 +1320,8 @@ const messages: TranslationMap = { 'settings.embeddings.testing': 'Wird getestet…', 'settings.embeddings.testSuccess': 'Verbunden — {dims} Dimensionen', 'settings.embeddings.connectionTestFailed': 'Test fehlgeschlagen', + 'settings.embeddings.verifyFallback': + 'Der Embeddings-Endpunkt konnte nicht überprüft werden. Stelle sicher, dass er läuft und ein Embedding-Modell bereitstellt, und speichere erneut.', 'settings.embeddings.testFailed': 'Fehlgeschlagen: {error}', 'settings.embeddings.saving': 'Wird gespeichert…', 'settings.embeddings.saved': 'Gespeichert.', diff --git a/app/src/lib/i18n/en.ts b/app/src/lib/i18n/en.ts index e4e07e1db..ab344f082 100644 --- a/app/src/lib/i18n/en.ts +++ b/app/src/lib/i18n/en.ts @@ -1638,6 +1638,8 @@ const en: TranslationMap = { 'settings.embeddings.testing': 'Testing…', 'settings.embeddings.testSuccess': 'Connected — {dims} dimensions', 'settings.embeddings.connectionTestFailed': 'Test failed', + 'settings.embeddings.verifyFallback': + "Couldn't verify the embeddings endpoint. Make sure it's running and serving an embedding model, then save again.", 'settings.embeddings.testFailed': 'Failed: {error}', 'settings.embeddings.saving': 'Saving…', 'settings.embeddings.saved': 'Saved.', diff --git a/app/src/lib/i18n/es.ts b/app/src/lib/i18n/es.ts index c422a1c99..987669776 100644 --- a/app/src/lib/i18n/es.ts +++ b/app/src/lib/i18n/es.ts @@ -1313,6 +1313,8 @@ const messages: TranslationMap = { 'settings.embeddings.testing': 'Probando…', 'settings.embeddings.testSuccess': 'Conectado — {dims} dimensiones', 'settings.embeddings.connectionTestFailed': 'La prueba falló', + 'settings.embeddings.verifyFallback': + 'No se pudo verificar el endpoint de embeddings. Asegúrate de que esté en ejecución y sirviendo un modelo de embeddings, luego guarda de nuevo.', 'settings.embeddings.testFailed': 'Fallido: {error}', 'settings.embeddings.saving': 'Guardando…', 'settings.embeddings.saved': 'Guardado.', diff --git a/app/src/lib/i18n/fr.ts b/app/src/lib/i18n/fr.ts index ebd75c358..408b578c1 100644 --- a/app/src/lib/i18n/fr.ts +++ b/app/src/lib/i18n/fr.ts @@ -1320,6 +1320,8 @@ const messages: TranslationMap = { 'settings.embeddings.testing': 'Test en cours…', 'settings.embeddings.testSuccess': 'Connecté — {dims} dimensions', 'settings.embeddings.connectionTestFailed': 'Test échoué', + 'settings.embeddings.verifyFallback': + "Impossible de vérifier le point de terminaison d'embeddings. Vérifiez qu'il fonctionne et qu'il propose un modèle d'embedding, puis enregistrez à nouveau.", 'settings.embeddings.testFailed': 'Échec : {error}', 'settings.embeddings.saving': 'Enregistrement…', 'settings.embeddings.saved': 'Enregistré.', diff --git a/app/src/lib/i18n/hi.ts b/app/src/lib/i18n/hi.ts index 6fa21abac..37c332f32 100644 --- a/app/src/lib/i18n/hi.ts +++ b/app/src/lib/i18n/hi.ts @@ -1284,6 +1284,8 @@ const messages: TranslationMap = { 'settings.embeddings.testing': 'परीक्षण हो रहा है…', 'settings.embeddings.testSuccess': 'कनेक्ट — {dims} आयाम', 'settings.embeddings.connectionTestFailed': 'परीक्षण विफल', + 'settings.embeddings.verifyFallback': + 'एम्बेडिंग एंडपॉइंट सत्यापित नहीं किया जा सका। सुनिश्चित करें कि यह चल रहा है और एक एम्बेडिंग मॉडल प्रदान कर रहा है, फिर दोबारा सहेजें।', 'settings.embeddings.testFailed': 'विफल: {error}', 'settings.embeddings.saving': 'सहेजा जा रहा है…', 'settings.embeddings.saved': 'सहेजा गया।', diff --git a/app/src/lib/i18n/id.ts b/app/src/lib/i18n/id.ts index ecc1f270f..a64e001e7 100644 --- a/app/src/lib/i18n/id.ts +++ b/app/src/lib/i18n/id.ts @@ -1293,6 +1293,8 @@ const messages: TranslationMap = { 'settings.embeddings.testing': 'Menguji…', 'settings.embeddings.testSuccess': 'Terhubung — {dims} dimensi', 'settings.embeddings.connectionTestFailed': 'Pengujian gagal', + 'settings.embeddings.verifyFallback': + 'Tidak dapat memverifikasi endpoint embeddings. Pastikan endpoint berjalan dan menyediakan model embedding, lalu simpan lagi.', 'settings.embeddings.testFailed': 'Gagal: {error}', 'settings.embeddings.saving': 'Menyimpan…', 'settings.embeddings.saved': 'Tersimpan.', diff --git a/app/src/lib/i18n/it.ts b/app/src/lib/i18n/it.ts index 67cc88975..404c3363c 100644 --- a/app/src/lib/i18n/it.ts +++ b/app/src/lib/i18n/it.ts @@ -1311,6 +1311,8 @@ const messages: TranslationMap = { 'settings.embeddings.testing': 'Test in corso…', 'settings.embeddings.testSuccess': 'Connesso — {dims} dimensioni', 'settings.embeddings.connectionTestFailed': 'Test non riuscito', + 'settings.embeddings.verifyFallback': + "Impossibile verificare l'endpoint di embedding. Assicurati che sia in esecuzione e fornisca un modello di embedding, poi salva di nuovo.", 'settings.embeddings.testFailed': 'Fallito: {error}', 'settings.embeddings.saving': 'Salvataggio…', 'settings.embeddings.saved': 'Salvato.', diff --git a/app/src/lib/i18n/ko.ts b/app/src/lib/i18n/ko.ts index 261f9dad1..62c418771 100644 --- a/app/src/lib/i18n/ko.ts +++ b/app/src/lib/i18n/ko.ts @@ -1281,6 +1281,8 @@ const messages: TranslationMap = { 'settings.embeddings.testing': '테스트 중…', 'settings.embeddings.testSuccess': '연결됨 — {dims} 차원', 'settings.embeddings.connectionTestFailed': '테스트 실패', + 'settings.embeddings.verifyFallback': + '임베딩 엔드포인트를 확인할 수 없습니다. 엔드포인트가 실행 중이고 임베딩 모델을 제공하는지 확인한 후 다시 저장하세요.', 'settings.embeddings.testFailed': '실패: {error}', 'settings.embeddings.saving': '저장 중…', 'settings.embeddings.saved': '저장됨.', diff --git a/app/src/lib/i18n/pl.ts b/app/src/lib/i18n/pl.ts index 9b30cdb3f..16dc3cb13 100644 --- a/app/src/lib/i18n/pl.ts +++ b/app/src/lib/i18n/pl.ts @@ -1303,6 +1303,8 @@ const messages: TranslationMap = { 'settings.embeddings.testing': 'Testowanie…', 'settings.embeddings.testSuccess': 'Połączono — wymiarów: {dims}', 'settings.embeddings.connectionTestFailed': 'Test nie powiódł się', + 'settings.embeddings.verifyFallback': + 'Nie można zweryfikować punktu końcowego osadzeń. Upewnij się, że działa i udostępnia model osadzeń, a następnie zapisz ponownie.', 'settings.embeddings.testFailed': 'Niepowodzenie: {error}', 'settings.embeddings.saving': 'Zapisywanie…', 'settings.embeddings.saved': 'Zapisano.', diff --git a/app/src/lib/i18n/pt.ts b/app/src/lib/i18n/pt.ts index 576fc1a76..2dc21b516 100644 --- a/app/src/lib/i18n/pt.ts +++ b/app/src/lib/i18n/pt.ts @@ -1317,6 +1317,8 @@ const messages: TranslationMap = { 'settings.embeddings.testing': 'Testando…', 'settings.embeddings.testSuccess': 'Conectado — {dims} dimensões', 'settings.embeddings.connectionTestFailed': 'Teste falhou', + 'settings.embeddings.verifyFallback': + 'Não foi possível verificar o endpoint de embeddings. Verifique se ele está em execução e fornecendo um modelo de embedding e salve novamente.', 'settings.embeddings.testFailed': 'Falhou: {error}', 'settings.embeddings.saving': 'Salvando…', 'settings.embeddings.saved': 'Salvo.', diff --git a/app/src/lib/i18n/ru.ts b/app/src/lib/i18n/ru.ts index 7b24cd579..f7c1d95ed 100644 --- a/app/src/lib/i18n/ru.ts +++ b/app/src/lib/i18n/ru.ts @@ -1301,6 +1301,8 @@ const messages: TranslationMap = { 'settings.embeddings.testing': 'Проверка…', 'settings.embeddings.testSuccess': 'Подключено — {dims} измерений', 'settings.embeddings.connectionTestFailed': 'Проверка не удалась', + 'settings.embeddings.verifyFallback': + 'Не удалось проверить конечную точку эмбеддингов. Убедитесь, что она запущена и предоставляет модель эмбеддингов, затем сохраните снова.', 'settings.embeddings.testFailed': 'Ошибка: {error}', 'settings.embeddings.saving': 'Сохранение…', 'settings.embeddings.saved': 'Сохранено.', diff --git a/app/src/lib/i18n/zh-CN.ts b/app/src/lib/i18n/zh-CN.ts index b928db921..caad352a7 100644 --- a/app/src/lib/i18n/zh-CN.ts +++ b/app/src/lib/i18n/zh-CN.ts @@ -1224,6 +1224,8 @@ const messages: TranslationMap = { 'settings.embeddings.testing': '测试中…', 'settings.embeddings.testSuccess': '已连接 — {dims} 维度', 'settings.embeddings.connectionTestFailed': '测试失败', + 'settings.embeddings.verifyFallback': + '无法验证嵌入端点。请确认其正在运行并提供嵌入模型,然后重新保存。', 'settings.embeddings.testFailed': '失败:{error}', 'settings.embeddings.saving': '保存中…', 'settings.embeddings.saved': '已保存。', diff --git a/app/src/services/api/embeddingsApi.ts b/app/src/services/api/embeddingsApi.ts index a11f11771..29b6045d6 100644 --- a/app/src/services/api/embeddingsApi.ts +++ b/app/src/services/api/embeddingsApi.ts @@ -44,6 +44,8 @@ export interface EmbeddingsUpdateResult { /** Present when confirm_wipe was required but not supplied */ error?: string; message?: string; + /** Underlying probe failure (HTTP status / server error body) for diagnosis (#4056) */ + detail?: string; old_signature?: string; } diff --git a/src/openhuman/embeddings/factory.rs b/src/openhuman/embeddings/factory.rs index 27cbf9940..8eec9f635 100644 --- a/src/openhuman/embeddings/factory.rs +++ b/src/openhuman/embeddings/factory.rs @@ -16,7 +16,7 @@ use super::{NoopEmbedding, OllamaEmbedding, OpenAiEmbedding}; /// arbitrary OpenAI-compatible servers (vLLM, text-embeddings-inference, /// stricter LocalAI builds) makes those servers 400 on an unknown field, so we /// gate on the model id rather than the provider kind. (Reviewer sanil-23, #3076.) -fn model_supports_dimensions(model: &str) -> bool { +pub(crate) fn model_supports_dimensions(model: &str) -> bool { model.starts_with("text-embedding-3-") } diff --git a/src/openhuman/embeddings/mod.rs b/src/openhuman/embeddings/mod.rs index 0721057fb..86c02db6d 100644 --- a/src/openhuman/embeddings/mod.rs +++ b/src/openhuman/embeddings/mod.rs @@ -40,6 +40,10 @@ pub use factory::{ create_embedding_provider, create_embedding_provider_with_credentials, default_embedding_provider, default_local_embedding_provider, }; +// `pub(crate)` helper — reused by the memory-tree OpenAI-compat adapter to gate +// configs whose dimension the fixed-1024 tree can't store (#4056). Not part of +// the public surface, so it can't ride the `pub use` above (E0364). +pub(crate) use factory::model_supports_dimensions; // #002 FR-015: the memory-tree OpenAI-compat embedder reuses the same key // resolution the embeddings RPC uses, so there is one source of truth. pub use noop::NoopEmbedding; diff --git a/src/openhuman/embeddings/openai_tests.rs b/src/openhuman/embeddings/openai_tests.rs index ee26243e9..ba906ce2f 100644 --- a/src/openhuman/embeddings/openai_tests.rs +++ b/src/openhuman/embeddings/openai_tests.rs @@ -525,6 +525,28 @@ async fn embed_dimension_mismatch() { assert!(err.to_string().contains("dimension mismatch")); } +/// Issue #4056: a `dims == 0` provider is the dimension-agnostic verification +/// probe — it must NOT enforce any length, so an endpoint returning its own +/// native size passes instead of being rejected. This is what lets a Custom +/// endpoint verify when the user's guessed `dimensions` differs from the +/// model's native output; the caller then adopts the returned length. +#[tokio::test] +async fn embed_dims_zero_skips_dimension_guard() { + let app = Router::new().route( + "/v1/embeddings", + post(|| async { + Json(serde_json::json!({ + "data": [{ "embedding": [1.0, 2.0, 3.0, 4.0, 5.0] }] + })) + }), + ); + let url = start_mock(app).await; + let p = OpenAiEmbedding::new(&url, "k", "m", 0); + + let result = p.embed(&["hi"]).await.unwrap(); + assert_eq!(result[0].len(), 5, "dims=0 must accept the native length"); +} + #[tokio::test] async fn embed_malformed_json() { let app = Router::new().route( diff --git a/src/openhuman/embeddings/rpc.rs b/src/openhuman/embeddings/rpc.rs index 54fbdc811..609f54545 100644 --- a/src/openhuman/embeddings/rpc.rs +++ b/src/openhuman/embeddings/rpc.rs @@ -7,10 +7,51 @@ use crate::openhuman::credentials::AuthService; use crate::rpc::RpcOutcome; use super::catalog; -use super::factory::create_embedding_provider_with_credentials; +use super::factory::{create_embedding_provider_with_credentials, model_supports_dimensions}; const LOG_PREFIX: &str = "[embeddings::rpc]"; +/// Dimension to run a Custom (OpenAI-compatible) verification probe at. +/// +/// The user-entered `dimensions` field is a guess: for any model outside the +/// `text-embedding-3-*` family we never send the OpenAI `dimensions` request +/// param (see [`model_supports_dimensions`]), so the endpoint returns its own +/// native vector length. Forcing the probe to enforce the guessed length makes +/// every reachable, valid embedding endpoint fail verification whenever the +/// guess (default 1024) differs from the native size — the root cause of +/// issue #4056. +/// +/// So we probe a `text-embedding-3-*` model at the configured size (the server +/// honours the param and returns exactly that), but probe every other model at +/// `0`, which disables both the request param and the post-response length +/// guard in `OpenAiEmbedding::embed` — the probe then only has to prove the +/// endpoint can embed, and we learn the real dimension from the returned +/// vector (see [`final_probe_dims`]). +fn probe_dims_for(model: &str, configured: usize) -> usize { + if model_supports_dimensions(model) { + configured + } else { + 0 + } +} + +/// Dimension to persist after a successful Custom verification probe. +/// +/// For a `text-embedding-3-*` model the endpoint honoured the requested size, +/// so keep the user's `configured` value (Matryoshka). For every other model we +/// probed dimension-agnostically, so adopt the endpoint's actual returned +/// length (`actual`) — the user can't be expected to know it, and storing the +/// real size is what lets the live embed path's length guard pass afterwards. +/// Falls back to `configured` if the probe somehow reported a zero-length +/// vector (defensive — `classify_embed_probe` already rejects empty vectors). +fn final_probe_dims(model: &str, configured: usize, actual: usize) -> usize { + if model_supports_dimensions(model) || actual == 0 { + configured + } else { + actual + } +} + /// Returns the current embedding settings plus the provider catalog. pub async fn get_settings(config: &Config) -> Result, String> { let provider = &config.memory.embedding_provider; @@ -102,12 +143,15 @@ pub async fn update_settings( let new_model = model .clone() .unwrap_or_else(|| config.memory.embedding_model.clone()); - let new_dims = dimensions.unwrap_or(config.memory.embedding_dimensions); - let new_sig = format_embedding_signature(&new_provider, &new_model, new_dims); + // `new_dims`/`new_sig`/`dims_changed` are recomputed after the Custom + // verification probe auto-detects the endpoint's real vector length + // (issue #4056), so they must be mutable. + let mut new_dims = dimensions.unwrap_or(config.memory.embedding_dimensions); + let mut new_sig = format_embedding_signature(&new_provider, &new_model, new_dims); let old_dims = config.memory.embedding_dimensions; - let dims_changed = new_dims != old_dims; - let sig_changed = new_sig != old_sig; + let mut dims_changed = new_dims != old_dims; + let mut sig_changed = new_sig != old_sig; // Setup-time verification gate (TAURI-RUST-5JR / 4P4): a Custom // (OpenAI-compatible) embeddings endpoint — e.g. LM Studio — must prove it @@ -131,11 +175,16 @@ pub async fn update_settings( _ => new_provider.clone(), }; if effective_provider.starts_with("custom:") { - match build_embedder(&config, &effective_provider, &new_model, new_dims) { + // Probe dimension-agnostically for non-`text-embedding-3-*` models so the + // user's guessed `dimensions` can't fail an otherwise-valid endpoint; the + // real length is detected from the returned vector below (issue #4056). + let probe_dims = probe_dims_for(&new_model, new_dims); + match build_embedder(&config, &effective_provider, &new_model, probe_dims) { Ok(embedder) => { // Time-box the probe so a black-hole host can't hang the RPC. tracing::debug!( provider = effective_provider.as_str(), + probe_dims, "{LOG_PREFIX} update_settings verifying embeddings endpoint with a test embed" ); let probe = tokio::time::timeout( @@ -150,6 +199,12 @@ pub async fn update_settings( Ok(Err(e)) => EmbedProbe::Failed(e.to_string()), Err(_elapsed) => EmbedProbe::TimedOut, }; + // Peek the actual vector length before the policy consumes the + // outcome — on a pass this is the endpoint's real dimension. + let probe_actual_dims = match &outcome { + EmbedProbe::Returned(vectors) => vectors.first().map(|v| v.len()).unwrap_or(0), + _ => 0, + }; if let Some(reject) = classify_embed_probe(outcome) { tracing::warn!( provider = effective_provider.as_str(), @@ -207,8 +262,28 @@ pub async fn update_settings( } return Ok(reject); } + // Passed. Adopt the endpoint's real vector length for every model + // we probed dimension-agnostically — the user can't be expected to + // know it, and storing the actual size is what keeps the live embed + // path's length guard from rejecting future embeds (issue #4056). + // `text-embedding-3-*` keeps the requested size (server honoured it). + let detected_dims = final_probe_dims(&new_model, new_dims, probe_actual_dims); + if detected_dims != new_dims { + tracing::info!( + provider = effective_provider.as_str(), + model = new_model.as_str(), + requested = new_dims, + detected = detected_dims, + "{LOG_PREFIX} update_settings auto-detected custom embedding dimension from probe" + ); + new_dims = detected_dims; + new_sig = format_embedding_signature(&new_provider, &new_model, new_dims); + dims_changed = new_dims != old_dims; + sig_changed = new_sig != old_sig; + } tracing::debug!( provider = effective_provider.as_str(), + new_dims, "{LOG_PREFIX} update_settings test embed passed — accepting config" ); } @@ -262,9 +337,11 @@ pub async fn update_settings( if let Some(m) = &model { config.memory.embedding_model = m.clone(); } - if let Some(d) = dimensions { - config.memory.embedding_dimensions = d; - } + // Persist `new_dims`, not the raw `dimensions` arg: the Custom verification + // probe may have auto-detected the endpoint's real length (issue #4056), and + // `new_dims` already defaults to the stored value when neither a new arg nor + // detection changed it — so this is a no-op for the unchanged case. + config.memory.embedding_dimensions = new_dims; if let Some(rl) = rate_limit_per_min { config.memory.embedding_rate_limit_per_min = rl; } @@ -443,10 +520,21 @@ pub async fn test_connection( slug }; + // Probe a Custom endpoint dimension-agnostically (issue #4056): the user's + // `dims` is a guess, so enforcing it here would make a valid endpoint fail + // the Test-connection button whenever the guess differs from the native + // size. Catalog providers keep their fixed `dims`. We still report the + // requested vs actual dimensions in the payload below. + let probe_dims = if provider_tag == "custom" { + probe_dims_for(model, dims) + } else { + dims + }; + let embedder = create_embedding_provider_with_credentials( provider_tag, model, - dims, + probe_dims, &api_key, custom_endpoint.as_deref(), ) @@ -456,6 +544,7 @@ pub async fn test_connection( provider = provider_tag, model, dims, + probe_dims, "{LOG_PREFIX} test_connection starting" ); @@ -790,6 +879,39 @@ mod tests { ); } + /// Issue #4056: a Custom endpoint is probed dimension-agnostically for any + /// model that doesn't honour the OpenAI `dimensions` request param, so the + /// user's guessed size can't fail an otherwise-valid endpoint. Only the + /// `text-embedding-3-*` family (which honours the param) is probed at the + /// requested size. + #[test] + fn probe_dims_for_zeroes_non_matryoshka_models() { + // text-embedding-3-* honours the param → probe at the requested size. + assert_eq!(probe_dims_for("text-embedding-3-large", 1024), 1024); + assert_eq!(probe_dims_for("text-embedding-3-small", 512), 512); + // Everything else → 0 (no param sent, no length guard). + assert_eq!(probe_dims_for("bge-m3", 1024), 0); + assert_eq!(probe_dims_for("nomic-embed-text", 768), 0); + assert_eq!(probe_dims_for("gpt-5-mini", 1024), 0); + } + + /// Issue #4056: after a successful probe we adopt the endpoint's real + /// returned length for auto-detected models, but keep the requested size for + /// `text-embedding-3-*` (the server returned exactly that). A zero actual + /// (defensive — empty vectors are already rejected upstream) falls back to + /// the configured value. + #[test] + fn final_probe_dims_adopts_actual_for_auto_detected_models() { + // Auto-detected model → adopt the real length, ignoring the guess. + assert_eq!(final_probe_dims("bge-m3", 1024, 1024), 1024); + assert_eq!(final_probe_dims("bge-m3", 1024, 768), 768); + assert_eq!(final_probe_dims("nomic-embed-text", 1024, 768), 768); + // text-embedding-3-* → keep the requested size (param was honoured). + assert_eq!(final_probe_dims("text-embedding-3-large", 1024, 3072), 1024); + // Defensive: zero actual falls back to the configured value. + assert_eq!(final_probe_dims("bge-m3", 1024, 0), 1024); + } + #[test] fn normalize_embed_model_id_strips_prefix_and_tag() { assert_eq!(normalize_embed_model_id("text-embedding-bge-m3"), "bge-m3"); diff --git a/src/openhuman/memory_tree/score/embed/openai_compat.rs b/src/openhuman/memory_tree/score/embed/openai_compat.rs index 9a5296c07..3a72fc4e0 100644 --- a/src/openhuman/memory_tree/score/embed/openai_compat.rs +++ b/src/openhuman/memory_tree/score/embed/openai_compat.rs @@ -139,6 +139,26 @@ impl OpenAiCompatEmbedder { } }; + // The memory tree's on-disk format is fixed at [`EMBEDDING_DIM`]. Models + // that don't honour the OpenAI `dimensions` request param (everything + // outside `text-embedding-3-*`) return their own native length, so a + // config whose stored dimension isn't `EMBEDDING_DIM` can never satisfy + // the tree. Building the adapter anyway would only defer the failure to + // the first embed ("expected 1024, got N") — refuse it here with an + // actionable message instead (Codex review on #4056). `text-embedding-3-*` + // is exempt: we request `EMBEDDING_DIM` below and the server reduces to it. + if !crate::openhuman::embeddings::model_supports_dimensions(model) + && config.memory.embedding_dimensions != EMBEDDING_DIM + { + anyhow::bail!( + "embeddings provider '{provider}' (model '{model}') produces \ + {}-dimensional vectors, but the memory tree requires {EMBEDDING_DIM}. \ + Choose a {EMBEDDING_DIM}-dimension model — an OpenAI `text-embedding-3-*` \ + model, or a {EMBEDDING_DIM}-dim model such as `mxbai-embed-large` or `bge-large`.", + config.memory.embedding_dimensions + ); + } + let inner = crate::openhuman::embeddings::create_embedding_provider_with_credentials( slug, model, @@ -329,4 +349,57 @@ mod tests { assert!(got.is_none(), "{p} must fall through, got Some"); } } + + /// Codex review on #4056: a custom config whose stored dimension isn't the + /// tree's fixed [`EMBEDDING_DIM`] (and whose model can't reduce to it via the + /// OpenAI `dimensions` param) must be refused at construction with a clear, + /// actionable error — not built and then failed at the first embed with a raw + /// "expected 1024, got N". This is what keeps an auto-detected non-1024 custom + /// endpoint (which the embeddings RPC still accepts) out of the 1024-only tree. + #[test] + fn err_for_non_reducible_model_with_incompatible_dimension() { + let (_tmp, mut cfg) = cfg_with_provider("custom:https://embed.example/v1"); + cfg.memory.embedding_model = "nomic-embed-text".to_string(); // not text-embedding-3-* + cfg.memory.embedding_dimensions = 768; // != EMBEDDING_DIM (1024) + // `expect_err` would require the Ok type (the embedder) to impl Debug, + // which it can't (boxed trait object) — match instead. + let err = match OpenAiCompatEmbedder::try_from_config(&cfg) { + Err(e) => e, + Ok(_) => panic!("768 != tree dim must error, got Ok"), + }; + let msg = format!("{err:#}"); + assert!( + msg.contains("768") && msg.contains(&EMBEDDING_DIM.to_string()), + "error must name both the model's dim and the required dim: {msg}" + ); + } + + /// A non-reducible model that natively matches [`EMBEDDING_DIM`] still builds — + /// only an incompatible dimension is refused. + #[test] + fn some_for_non_reducible_model_at_tree_dimension() { + let (_tmp, mut cfg) = cfg_with_provider("custom:https://embed.example/v1"); + cfg.memory.embedding_model = "mxbai-embed-large".to_string(); + cfg.memory.embedding_dimensions = EMBEDDING_DIM; // 1024 + let got = OpenAiCompatEmbedder::try_from_config(&cfg).expect("no error"); + assert!( + got.is_some(), + "a 1024-native custom model must build the tree adapter" + ); + } + + /// `text-embedding-3-*` is exempt from the dimension guard: the adapter + /// requests `EMBEDDING_DIM` and the server reduces to it, so even a config + /// stored at a different dimension still builds. + #[test] + fn some_for_reducible_model_regardless_of_stored_dimension() { + let (_tmp, mut cfg) = cfg_with_provider("openai"); + cfg.memory.embedding_model = "text-embedding-3-large".to_string(); + cfg.memory.embedding_dimensions = 256; // reducible — tree still requests 1024 + let got = OpenAiCompatEmbedder::try_from_config(&cfg).expect("no error"); + assert!( + got.is_some(), + "reducible model must build regardless of stored dim" + ); + } }