diff --git a/app/src/lib/i18n/ar.ts b/app/src/lib/i18n/ar.ts index bf1657055..91da680b6 100644 --- a/app/src/lib/i18n/ar.ts +++ b/app/src/lib/i18n/ar.ts @@ -5201,6 +5201,8 @@ const messages: TranslationMap = { 'يتجاوز نموذج استخراج الذاكرة المهلة الزمنية، لذا فإن بنية الويكي قليلة. بدّل نموذج استخراج الذاكرة إلى نموذج أسرع في الإعدادات → الذكاء الاصطناعي.', 'memory.health.remediation.summarizer_unavailable': 'لا يتوفر مزوّد تلخيص لميزة إنشاء أشجار التلخيص. فعّل الذكاء الاصطناعي المحلي (Ollama)، أو فعّل تلخيص السحابة في الإعدادات → الذكاء الاصطناعي → الذاكرة.', + 'memory.health.remediation.empty_input_refused': + 'تم تخطي عنصر ذاكرة لأن نصه كان فارغًا. لا حاجة لأي إجراء — تستمر العناصر الجديدة في التضمين بشكل طبيعي.', 'memory.health.remediation.transient': 'حدث خطأ مؤقت أدى إلى مقاطعة معالجة الذاكرة. ستتم إعادة المحاولة تلقائيًا.', 'memory.health.remediation.unknown': diff --git a/app/src/lib/i18n/bn.ts b/app/src/lib/i18n/bn.ts index 837d4e40d..4c2b1c6e3 100644 --- a/app/src/lib/i18n/bn.ts +++ b/app/src/lib/i18n/bn.ts @@ -5307,6 +5307,8 @@ const messages: TranslationMap = { 'মেমরি এক্সট্র্যাকশন মডেল টাইম আউট হচ্ছে, তাই উইকিতে সামান্য কাঠামো আছে। সেটিংস → AI-তে মেমরি এক্সট্র্যাকশন মডেল একটি দ্রুততর মডেলে পরিবর্তন করুন।', 'memory.health.remediation.summarizer_unavailable': 'সারাংশ ট্রি তৈরির জন্য কোনও সারাংশ প্রদানকারী উপলব্ধ নেই। স্থানীয় AI (Ollama) সক্ষম করুন, অথবা সেটিংস → AI → মেমরিতে ক্লাউড সারাংশ সক্ষম করুন।', + 'memory.health.remediation.empty_input_refused': + 'একটি মেমরি আইটেম এড়িয়ে যাওয়া হয়েছে কারণ এর পাঠ্য খালি ছিল। কোনো পদক্ষেপের প্রয়োজন নেই — নতুন আইটেমগুলি স্বাভাবিকভাবে এমবেড করতে থাকে।', 'memory.health.remediation.transient': 'একটি অস্থায়ী ত্রুটি মেমরি প্রক্রিয়াকরণে বাধা দিয়েছে। স্বয়ংক্রিয়ভাবে পুনরায় চেষ্টা করা হবে।', 'memory.health.remediation.unknown': diff --git a/app/src/lib/i18n/de.ts b/app/src/lib/i18n/de.ts index 7e0fdc35c..95ea68919 100644 --- a/app/src/lib/i18n/de.ts +++ b/app/src/lib/i18n/de.ts @@ -5443,6 +5443,8 @@ const messages: TranslationMap = { 'Das Modell zur Speicherextraktion überschreitet die Zeit, daher hat das Wiki wenig Struktur. Wechsle das Modell für die Speicherextraktion unter Einstellungen → KI zu einem schnelleren.', 'memory.health.remediation.summarizer_unavailable': 'Für „Zusammenfassungsbäume erstellen” ist kein Zusammenfassungsanbieter verfügbar. Aktiviere die lokale KI (Ollama) oder aktiviere die Cloud-Zusammenfassung unter Einstellungen → KI → Speicher.', + 'memory.health.remediation.empty_input_refused': + 'Ein Speicherelement wurde übersprungen, weil sein Text leer war. Keine Aktion erforderlich — neue Einträge werden weiterhin normal eingebettet.', 'memory.health.remediation.transient': 'Ein vorübergehender Fehler hat die Speicherverarbeitung unterbrochen. Es wird automatisch erneut versucht.', 'memory.health.remediation.unknown': diff --git a/app/src/lib/i18n/en.ts b/app/src/lib/i18n/en.ts index 707742843..7a733c88a 100644 --- a/app/src/lib/i18n/en.ts +++ b/app/src/lib/i18n/en.ts @@ -825,6 +825,8 @@ const en: TranslationMap = { 'The memory extraction model is timing out, so the wiki has little structure. Switch the Memory extraction model to a faster one in Settings → AI.', 'memory.health.remediation.summarizer_unavailable': 'No summarization provider is available for Build Summary Trees. Enable local AI (Ollama), or enable cloud summarization in Settings → AI → Memory.', + 'memory.health.remediation.empty_input_refused': + 'A memory item was skipped because its text was empty. No action needed — newer items continue to embed normally.', 'memory.health.remediation.transient': 'A temporary error interrupted memory processing. It will retry automatically.', 'memory.health.remediation.unknown': diff --git a/app/src/lib/i18n/es.ts b/app/src/lib/i18n/es.ts index f87f16a08..3b59ab9b4 100644 --- a/app/src/lib/i18n/es.ts +++ b/app/src/lib/i18n/es.ts @@ -5411,6 +5411,8 @@ const messages: TranslationMap = { 'El modelo de extracción de memoria está agotando el tiempo de espera, por lo que la wiki tiene poca estructura. Cambia el modelo de extracción de memoria por uno más rápido en Configuración → IA.', 'memory.health.remediation.summarizer_unavailable': 'No hay ningún proveedor de resúmenes disponible para Crear árboles de resumen. Activa la IA local (Ollama) o activa el resumen en la nube en Configuración → IA → Memoria.', + 'memory.health.remediation.empty_input_refused': + 'Se omitió un elemento de memoria porque su texto estaba vacío. No se requiere ninguna acción — los elementos nuevos siguen incrustándose con normalidad.', 'memory.health.remediation.transient': 'Un error temporal interrumpió el procesamiento de la memoria. Se reintentará automáticamente.', 'memory.health.remediation.unknown': diff --git a/app/src/lib/i18n/fr.ts b/app/src/lib/i18n/fr.ts index aa851dae7..c5a8740c0 100644 --- a/app/src/lib/i18n/fr.ts +++ b/app/src/lib/i18n/fr.ts @@ -5431,6 +5431,8 @@ const messages: TranslationMap = { "Le modèle d'extraction de mémoire dépasse le délai imparti, le wiki a donc peu de structure. Choisissez un modèle d'extraction de mémoire plus rapide dans Paramètres → IA.", 'memory.health.remediation.summarizer_unavailable': "Aucun fournisseur de résumé n'est disponible pour Créer des arbres de résumé. Activez l'IA locale (Ollama) ou activez la synthèse cloud dans Paramètres → IA → Mémoire.", + 'memory.health.remediation.empty_input_refused': + "Un élément de mémoire a été ignoré car son texte était vide. Aucune action requise — les nouveaux éléments continuent de s'intégrer normalement.", 'memory.health.remediation.transient': 'Une erreur temporaire a interrompu le traitement de la mémoire. Une nouvelle tentative aura lieu automatiquement.', 'memory.health.remediation.unknown': diff --git a/app/src/lib/i18n/hi.ts b/app/src/lib/i18n/hi.ts index 258a5617e..a126f59ad 100644 --- a/app/src/lib/i18n/hi.ts +++ b/app/src/lib/i18n/hi.ts @@ -5310,6 +5310,8 @@ const messages: TranslationMap = { 'मेमोरी एक्सट्रैक्शन मॉडल टाइम आउट हो रहा है, इसलिए विकी में बहुत कम संरचना है। सेटिंग्स → AI में मेमोरी एक्सट्रैक्शन मॉडल को तेज़ मॉडल में बदलें।', 'memory.health.remediation.summarizer_unavailable': 'सारांश ट्री बनाएँ के लिए कोई सारांश प्रदाता उपलब्ध नहीं है। स्थानीय AI (Ollama) सक्षम करें, या सेटिंग्स → AI → मेमोरी में क्लाउड सारांश सक्षम करें।', + 'memory.health.remediation.empty_input_refused': + 'एक मेमोरी आइटम छोड़ दिया गया क्योंकि उसका टेक्स्ट खाली था। कोई कार्रवाई आवश्यक नहीं — नए आइटम सामान्य रूप से एम्बेड होते रहेंगे।', 'memory.health.remediation.transient': 'एक अस्थायी त्रुटि ने मेमोरी प्रोसेसिंग को बाधित किया। स्वचालित रूप से पुनः प्रयास किया जाएगा।', 'memory.health.remediation.unknown': diff --git a/app/src/lib/i18n/id.ts b/app/src/lib/i18n/id.ts index 658c81b24..23354d746 100644 --- a/app/src/lib/i18n/id.ts +++ b/app/src/lib/i18n/id.ts @@ -5323,6 +5323,8 @@ const messages: TranslationMap = { 'Model ekstraksi memori kehabisan waktu, sehingga wiki memiliki sedikit struktur. Ganti model ekstraksi memori ke yang lebih cepat di Pengaturan → AI.', 'memory.health.remediation.summarizer_unavailable': 'Tidak ada penyedia ringkasan yang tersedia untuk Buat Pohon Ringkasan. Aktifkan AI lokal (Ollama), atau aktifkan ringkasan cloud di Pengaturan → AI → Memori.', + 'memory.health.remediation.empty_input_refused': + 'Item memori dilewati karena teksnya kosong. Tidak diperlukan tindakan — item baru tetap disematkan seperti biasa.', 'memory.health.remediation.transient': 'Kesalahan sementara mengganggu pemrosesan memori. Akan dicoba lagi secara otomatis.', 'memory.health.remediation.unknown': diff --git a/app/src/lib/i18n/it.ts b/app/src/lib/i18n/it.ts index 969fb6fc1..cb956f7ee 100644 --- a/app/src/lib/i18n/it.ts +++ b/app/src/lib/i18n/it.ts @@ -5400,6 +5400,8 @@ const messages: TranslationMap = { 'Il modello di estrazione della memoria sta andando in timeout, quindi il wiki ha poca struttura. Passa a un modello di estrazione della memoria più veloce in Impostazioni → IA.', 'memory.health.remediation.summarizer_unavailable': "Nessun provider di riepilogo è disponibile per Crea alberi di riepilogo. Abilita l'IA locale (Ollama) o abilita il riepilogo cloud in Impostazioni → IA → Memoria.", + 'memory.health.remediation.empty_input_refused': + 'Un elemento di memoria è stato saltato perché il suo testo era vuoto. Nessuna azione necessaria — i nuovi elementi continuano a essere incorporati normalmente.', 'memory.health.remediation.transient': "Un errore temporaneo ha interrotto l'elaborazione della memoria. Verrà riprovato automaticamente.", 'memory.health.remediation.unknown': diff --git a/app/src/lib/i18n/ko.ts b/app/src/lib/i18n/ko.ts index f92eacec7..70ad2a82a 100644 --- a/app/src/lib/i18n/ko.ts +++ b/app/src/lib/i18n/ko.ts @@ -5256,6 +5256,8 @@ const messages: TranslationMap = { '메모리 추출 모델이 시간 초과되어 위키 구조가 거의 없습니다. 설정 → AI에서 메모리 추출 모델을 더 빠른 것으로 변경하세요.', 'memory.health.remediation.summarizer_unavailable': '요약 트리 만들기에 사용할 수 있는 요약 제공자가 없습니다. 로컬 AI(Ollama)를 활성화하거나, 설정 → AI → 메모리에서 클라우드 요약을 활성화하세요.', + 'memory.health.remediation.empty_input_refused': + '텍스트가 비어 있어 메모리 항목이 건너뛰어졌습니다. 조치가 필요하지 않습니다 — 새 항목은 정상적으로 임베딩됩니다.', 'memory.health.remediation.transient': '일시적인 오류로 메모리 처리가 중단되었습니다. 자동으로 다시 시도됩니다.', 'memory.health.remediation.unknown': diff --git a/app/src/lib/i18n/pl.ts b/app/src/lib/i18n/pl.ts index af115d1a8..5773cf662 100644 --- a/app/src/lib/i18n/pl.ts +++ b/app/src/lib/i18n/pl.ts @@ -5392,6 +5392,8 @@ const messages: TranslationMap = { 'Model ekstrakcji pamięci przekracza limit czasu, więc wiki ma niewielką strukturę. Zmień model ekstrakcji pamięci na szybszy w Ustawienia → AI.', 'memory.health.remediation.summarizer_unavailable': 'Brak dostępnego dostawcy podsumowań dla funkcji Twórz drzewa podsumowań. Włącz lokalną AI (Ollama) lub włącz podsumowywanie w chmurze w Ustawienia → AI → Pamięć.', + 'memory.health.remediation.empty_input_refused': + 'Pominięto element pamięci, ponieważ jego tekst był pusty. Żadne działanie nie jest wymagane — nowe elementy są nadal osadzane normalnie.', 'memory.health.remediation.transient': 'Tymczasowy błąd przerwał przetwarzanie pamięci. Ponowna próba nastąpi automatycznie.', 'memory.health.remediation.unknown': diff --git a/app/src/lib/i18n/pt.ts b/app/src/lib/i18n/pt.ts index b3ba8fca4..97f2bd386 100644 --- a/app/src/lib/i18n/pt.ts +++ b/app/src/lib/i18n/pt.ts @@ -5395,6 +5395,8 @@ const messages: TranslationMap = { 'O modelo de extração de memória está expirando o tempo limite, então o wiki tem pouca estrutura. Mude o modelo de extração de memória para um mais rápido em Configurações → IA.', 'memory.health.remediation.summarizer_unavailable': 'Nenhum provedor de resumo está disponível para Criar árvores de resumo. Ative a IA local (Ollama) ou ative o resumo na nuvem em Configurações → IA → Memória.', + 'memory.health.remediation.empty_input_refused': + 'Um item de memória foi ignorado porque o texto estava vazio. Nenhuma ação necessária — itens novos continuam a ser incorporados normalmente.', 'memory.health.remediation.transient': 'Um erro temporário interrompeu o processamento da memória. Será repetido automaticamente.', 'memory.health.remediation.unknown': diff --git a/app/src/lib/i18n/ru.ts b/app/src/lib/i18n/ru.ts index 7653c0641..0865a619a 100644 --- a/app/src/lib/i18n/ru.ts +++ b/app/src/lib/i18n/ru.ts @@ -5361,6 +5361,8 @@ const messages: TranslationMap = { 'Модель извлечения памяти превышает время ожидания, поэтому в вики мало структуры. Выберите более быструю модель извлечения памяти в разделе Настройки → ИИ.', 'memory.health.remediation.summarizer_unavailable': 'Нет доступного поставщика суммаризации для «Построить деревья сводок». Включите локальный ИИ (Ollama) или включите облачную суммаризацию в разделе Настройки → ИИ → Память.', + 'memory.health.remediation.empty_input_refused': + 'Элемент памяти пропущен, так как его текст был пуст. Действия не требуются — новые элементы продолжают встраиваться как обычно.', 'memory.health.remediation.transient': 'Временная ошибка прервала обработку памяти. Повтор произойдёт автоматически.', 'memory.health.remediation.unknown': diff --git a/app/src/lib/i18n/zh-CN.ts b/app/src/lib/i18n/zh-CN.ts index a46357be0..1c3cfc1e5 100644 --- a/app/src/lib/i18n/zh-CN.ts +++ b/app/src/lib/i18n/zh-CN.ts @@ -5038,6 +5038,8 @@ const messages: TranslationMap = { '记忆提取模型超时,因此 Wiki 结构很少。请在设置 → AI 中将记忆提取模型更换为更快的模型。', 'memory.health.remediation.summarizer_unavailable': '没有可用于构建摘要树的摘要提供方。请启用本地 AI(Ollama),或在设置 → AI → 记忆中启用云端摘要。', + 'memory.health.remediation.empty_input_refused': + '由于文本为空,一项记忆已被跳过。无需操作 — 新条目继续正常嵌入。', 'memory.health.remediation.transient': '临时错误中断了记忆处理。将自动重试。', 'memory.health.remediation.unknown': '记忆处理遇到问题。请在设置 → AI 中检查配置。', // Chat — agent-generated artifacts (#2779) diff --git a/src/openhuman/agent/harness/archivist/lifecycle.rs b/src/openhuman/agent/harness/archivist/lifecycle.rs index 79683f180..845803cdc 100644 --- a/src/openhuman/agent/harness/archivist/lifecycle.rs +++ b/src/openhuman/agent/harness/archivist/lifecycle.rs @@ -320,55 +320,8 @@ impl ArchivistHook { } // ── Finalize-time embedding ─────────────────────────────────────── - // Embed the recap only when the segment is being finalized (closed). - // Never embed per-turn or on an open segment — this is the single - // write point for segment_embeddings rows. - if let Some(ref embedder) = self.embedder { - let model_signature = embedder.name().to_string(); - tracing::debug!( - "[archivist] embedding recap segment={} model={}", - segment.segment_id, - model_signature - ); - match embedder.embed(&summary).await { - Ok(vec) => { - match segments::segment_embedding_upsert( - conn, - &segment.segment_id, - &model_signature, - &vec, - now, - ) { - Ok(()) => { - tracing::debug!( - "[archivist] embedding stored segment={} model={} dim={}", - segment.segment_id, - model_signature, - vec.len() - ); - } - Err(e) => { - tracing::warn!( - "[archivist] failed to persist segment embedding (non-fatal) segment={}: {e}", - segment.segment_id - ); - } - } - } - Err(e) => { - tracing::warn!( - "[archivist] embed call failed (non-fatal) segment={} model={}: {e}", - segment.segment_id, - model_signature - ); - } - } - } else { - tracing::debug!( - "[archivist] no embedder — skipping segment embedding segment={}", - segment.segment_id - ); - } + self.embed_segment_recap(conn, &segment.segment_id, &summary, now) + .await; // ── Heuristic event extraction ──────────────────────────────────── if !segment_text.is_empty() { @@ -458,4 +411,66 @@ impl ArchivistHook { } } } + + /// Embed `summary` for `segment_id` and write the per-model embedding row. + /// + /// Embed the recap only when the segment is being finalized (closed). + /// Never embed per-turn or on an open segment — this is the single + /// write point for `segment_embeddings` rows. + /// + /// Skip when the recap is empty/whitespace — `summarize_entries` can + /// return "" (LLM error fallback + the user-turn filter above yielding + /// zero entries) and an empty embed input is guaranteed to 400 from + /// the upstream embedding API (#13021). The segment is sealed without + /// an embedding row; subsequent recap edits can re-embed. + pub(super) async fn embed_segment_recap( + &self, + conn: &Arc>, + segment_id: &str, + summary: &str, + now: f64, + ) { + if summary.trim().is_empty() { + tracing::warn!( + "[archivist] skipping embedding: recap is empty/whitespace segment={segment_id}" + ); + return; + } + let Some(ref embedder) = self.embedder else { + tracing::debug!( + "[archivist] no embedder — skipping segment embedding segment={segment_id}" + ); + return; + }; + let model_signature = embedder.name().to_string(); + tracing::debug!("[archivist] embedding recap segment={segment_id} model={model_signature}"); + match embedder.embed(summary).await { + Ok(vec) => { + match segments::segment_embedding_upsert( + conn, + segment_id, + &model_signature, + &vec, + now, + ) { + Ok(()) => { + tracing::debug!( + "[archivist] embedding stored segment={segment_id} model={model_signature} dim={}", + vec.len() + ); + } + Err(e) => { + tracing::warn!( + "[archivist] failed to persist segment embedding (non-fatal) segment={segment_id}: {e}" + ); + } + } + } + Err(e) => { + tracing::warn!( + "[archivist] embed call failed (non-fatal) segment={segment_id} model={model_signature}: {e}" + ); + } + } + } } diff --git a/src/openhuman/agent/harness/archivist_tests.rs b/src/openhuman/agent/harness/archivist_tests.rs index bed6b1a93..19b4c2d7b 100644 --- a/src/openhuman/agent/harness/archivist_tests.rs +++ b/src/openhuman/agent/harness/archivist_tests.rs @@ -892,3 +892,141 @@ async fn phase2_flush_also_triggers_tree_ingest() { "Expected ≥ 1 tree chunk after flush_open_segment triggers segment ingest; got {after}" ); } + +// ── #13021 empty/whitespace recap embed-skip guard ─────────────────────────── +// +// `on_segment_closed` defends against ever passing an empty or whitespace +// recap into the embedder by calling `embed_segment_recap`, which short- +// circuits before `Embedder::embed` runs. The skip is unreachable through +// the current `summarize_entries` call graph today (the heuristic +// `fallback_summary` always returns non-empty text), so these tests drive +// `embed_segment_recap` directly to lock the guard against future +// regressions where `summarize_entries` could return `""`. + +/// Embedder stub that panics if `embed` is invoked. Used to prove that +/// the empty-summary guard short-circuits BEFORE the upstream provider +/// is contacted (the very fault the #13021 fix prevents). +struct PanicOnEmbedEmbedder; + +#[async_trait::async_trait] +impl crate::openhuman::memory_tree::score::embed::Embedder for PanicOnEmbedEmbedder { + fn name(&self) -> &'static str { + "panic-embedder-v1" + } + + async fn embed(&self, text: &str) -> anyhow::Result> { + panic!( + "embed_segment_recap must not call Embedder::embed for empty/whitespace recap (got {text:?})" + ); + } +} + +fn hook_with_panic_embedder(conn: Arc>) -> ArchivistHook { + ArchivistHook::new_with_stubs( + conn, + Arc::new(StubChatProvider), + Arc::new(PanicOnEmbedEmbedder), + ) +} + +/// An empty recap must short-circuit before calling `Embedder::embed`, and +/// must not write a row to `segment_embeddings`. The segment row itself +/// remains intact (this helper does not touch segment status). +#[tokio::test] +async fn embed_segment_recap_skips_empty_summary() { + let conn = setup_conn(); + let hook = hook_with_panic_embedder(conn.clone()); + + let segment_id = "seg-empty-recap"; + seg::segment_create( + &conn, + segment_id, + "session-x", + "global", + 1, + Some(0), + 1.0, + 1.0, + ) + .unwrap(); + seg::segment_close(&conn, segment_id, 2.0).unwrap(); + + // PanicOnEmbedEmbedder would panic if Embedder::embed were invoked; + // reaching this point proves the guard short-circuited. + hook.embed_segment_recap(&conn, segment_id, "", 3.0).await; + + let row = seg::segment_embedding_get(&conn, segment_id, "panic-embedder-v1").unwrap(); + assert!( + row.is_none(), + "No embedding row should exist for an empty-recap segment" + ); +} + +/// Whitespace-only recaps (newlines, tabs, spaces) must also short-circuit +/// — the upstream provider rejects whitespace inputs the same way it +/// rejects empty inputs (#13021). +#[tokio::test] +async fn embed_segment_recap_skips_whitespace_summary() { + let conn = setup_conn(); + let hook = hook_with_panic_embedder(conn.clone()); + + let segment_id = "seg-ws-recap"; + seg::segment_create( + &conn, + segment_id, + "session-y", + "global", + 1, + Some(0), + 1.0, + 1.0, + ) + .unwrap(); + seg::segment_close(&conn, segment_id, 2.0).unwrap(); + + hook.embed_segment_recap(&conn, segment_id, " \n\t ", 3.0) + .await; + + let row = seg::segment_embedding_get(&conn, segment_id, "panic-embedder-v1").unwrap(); + assert!( + row.is_none(), + "No embedding row should exist for a whitespace-only recap segment" + ); +} + +/// Positive control: a non-empty recap is embedded and the row IS written. +/// Without this, the empty/whitespace tests above would pass even if the +/// guard erroneously skipped every recap. +#[tokio::test] +async fn embed_segment_recap_writes_row_for_non_empty_summary() { + let conn = setup_conn(); + let hook = hook_with_stubs(conn.clone()); + + let segment_id = "seg-ok-recap"; + seg::segment_create( + &conn, + segment_id, + "session-z", + "global", + 1, + Some(0), + 1.0, + 1.0, + ) + .unwrap(); + seg::segment_close(&conn, segment_id, 2.0).unwrap(); + + hook.embed_segment_recap(&conn, segment_id, "real recap text", 3.0) + .await; + + let row = seg::segment_embedding_get(&conn, segment_id, "stub-embedder-v1").unwrap(); + assert!( + row.is_some(), + "Embedding row should exist when recap is non-empty" + ); + assert_eq!( + row.unwrap().len(), + 4, + "Expected 4-dim vector from stub embedder" + ); +} diff --git a/src/openhuman/embeddings/cloud.rs b/src/openhuman/embeddings/cloud.rs index dece6e6f7..18675b518 100644 --- a/src/openhuman/embeddings/cloud.rs +++ b/src/openhuman/embeddings/cloud.rs @@ -116,6 +116,24 @@ impl EmbeddingProvider for OpenHumanCloudEmbedding { if texts.is_empty() { return Ok(Vec::new()); } + // Mirror the OpenAI provider's empty/whitespace guard *here* so we + // never resolve the bearer or build the URL for an input the backend + // will reject as `"input must be a non-empty string …"` (#13021). + // Cheaper, and keeps unauthenticated test contexts off the auth path. + if let Some(idx) = texts.iter().position(|t| t.trim().is_empty()) { + tracing::warn!( + target: "cloud::embed", + "[cloud] refusing embed: input[{idx}] is empty/whitespace \ + (count={}, model={}). Caller must filter empty strings.", + texts.len(), + self.model, + ); + anyhow::bail!( + "cloud embed: refusing empty/whitespace input at index {idx} of {} (model={})", + texts.len(), + self.model, + ); + } let token = self.resolve_bearer()?; let inner = OpenAiEmbedding::new(&self.base_url(), &token, &self.model, self.dims); inner.embed(texts).await @@ -167,4 +185,29 @@ mod tests { ); assert!(p.embed(&[]).await.unwrap().is_empty()); } + + #[tokio::test] + async fn embed_refuses_empty_string_without_auth() { + // The whitespace pre-flight (#13021) MUST fire before `resolve_bearer` + // — `secrets_encrypt = false` and no session means the AuthService + // would otherwise bail with `"No backend session for cloud + // embeddings…"` and mask the real defect. Asserting the bail wording + // here pins the order of the two checks. + let p = OpenHumanCloudEmbedding::new( + None, + None, + false, + DEFAULT_CLOUD_EMBEDDING_MODEL, + DEFAULT_CLOUD_EMBEDDING_DIMENSIONS, + ); + let err = p.embed(&[""]).await.unwrap_err().to_string(); + assert!( + err.contains("refusing empty/whitespace input at index 0"), + "expected pre-flight refusal, got: {err}" + ); + assert!( + !err.contains("No backend session"), + "guard must run before resolve_bearer, got: {err}" + ); + } } diff --git a/src/openhuman/embeddings/openai.rs b/src/openhuman/embeddings/openai.rs index a645c48bf..5ceefb90e 100644 --- a/src/openhuman/embeddings/openai.rs +++ b/src/openhuman/embeddings/openai.rs @@ -121,6 +121,28 @@ impl EmbeddingProvider for OpenAiEmbedding { return Ok(Vec::new()); } + // Pre-flight: empty / whitespace-only entries are guaranteed 400s from + // the upstream (OpenAI: `"input must be a non-empty string"`; OpenHuman + // cloud backend: `"input must be a non-empty string or array of + // non-empty strings"`). Bailing here keeps the round-trip and quota + // out of the picture and — crucially — bypasses the `report_error_or_ + // expected` Sentry route below, so a caller passing an empty summary + // stops manifesting as a server fault (#13021). + if let Some(idx) = texts.iter().position(|t| t.trim().is_empty()) { + tracing::warn!( + target: "openai::embed", + "[openai] refusing embed: input[{idx}] is empty/whitespace \ + (count={}, model={}). Caller must filter empty strings.", + texts.len(), + self.model, + ); + anyhow::bail!( + "openai embed: refusing empty/whitespace input at index {idx} of {} (model={})", + texts.len(), + self.model, + ); + } + let url = self.embeddings_url(); tracing::debug!( diff --git a/src/openhuman/embeddings/openai_tests.rs b/src/openhuman/embeddings/openai_tests.rs index f7b6490ee..0dce10181 100644 --- a/src/openhuman/embeddings/openai_tests.rs +++ b/src/openhuman/embeddings/openai_tests.rs @@ -124,6 +124,55 @@ async fn empty_input_returns_empty() { assert!(result.is_empty()); } +// ── empty/whitespace entries — pre-flight reject (#13021) ──────── +// +// `embed(&[""])` and friends used to fall through to the HTTP layer +// and trip a backend 400 ("input must be a non-empty string …"), +// which was then captured as a Sentry server fault even though the +// real defect was a caller passing empty text. The guard bails +// without touching the network — the "http://unused" base URL would +// otherwise refuse to connect. + +#[tokio::test] +async fn embed_refuses_single_empty_string() { + let p = OpenAiEmbedding::new("http://unused", "k", "m", 1); + let err = p.embed(&[""]).await.unwrap_err().to_string(); + assert!( + err.contains("refusing empty/whitespace input at index 0"), + "unexpected error: {err}" + ); +} + +#[tokio::test] +async fn embed_refuses_whitespace_only_string() { + let p = OpenAiEmbedding::new("http://unused", "k", "m", 1); + let err = p.embed(&[" \n\t"]).await.unwrap_err().to_string(); + assert!(err.contains("refusing empty/whitespace input at index 0")); +} + +#[tokio::test] +async fn embed_refuses_mixed_batch_with_empty() { + let p = OpenAiEmbedding::new("http://unused", "k", "m", 1); + let err = p.embed(&["ok", "", "fine"]).await.unwrap_err().to_string(); + assert!(err.contains("refusing empty/whitespace input at index 1")); +} + +#[tokio::test] +async fn embed_refuses_does_not_use_embedding_api_error_prefix() { + // The classifier in `core::observability` treats `"Embedding API error"` + // / `"("` shapes as upstream HTTP failures. The client-side + // pre-flight refusal MUST NOT collide with that shape, otherwise this + // very fix would re-enter the same Sentry-as-server-fault path that + // #13021 was about. Lock the bail wording so a future rename can't + // silently reintroduce the regression. + let p = OpenAiEmbedding::new("http://unused", "k", "m", 1); + let err = p.embed(&[""]).await.unwrap_err().to_string(); + assert!( + !err.contains("Embedding API error"), + "bail wording must not collide with TransientUpstreamHttp classifier: {err}" + ); +} + // ── embed — success ───────────────────────────────────── #[tokio::test] diff --git a/src/openhuman/memory_tree/health/mod.rs b/src/openhuman/memory_tree/health/mod.rs index 4fa1d0dff..d87a7637f 100644 --- a/src/openhuman/memory_tree/health/mod.rs +++ b/src/openhuman/memory_tree/health/mod.rs @@ -74,6 +74,13 @@ pub enum FailureCode { /// local path was selected; this covers the cloud-only setup whose provider /// failed to resolve, so the remediation names both paths. SummarizerUnavailable, + /// The embedding provider refused an empty/whitespace input at the + /// pre-flight guard (#13021). Unrecoverable per-row: the offending row + /// will never become embeddable, so the worker must tombstone it instead + /// of retrying. Bail wording for both `OpenAiEmbedding::embed` and + /// `OpenHumanCloudEmbedding::embed` starts with + /// `" embed: refusing empty/whitespace input ..."`. + EmptyInputRefused, /// Catch-all transient failure (network 5xx, timeout, truncated JSON). Transient, } @@ -90,6 +97,7 @@ impl FailureCode { Self::LocalModelUnavailable => "local_model_unavailable", Self::ExtractionTimeout => "extraction_timeout", Self::SummarizerUnavailable => "summarizer_unavailable", + Self::EmptyInputRefused => "empty_input_refused", Self::Transient => "transient", } } @@ -104,6 +112,7 @@ impl FailureCode { "local_model_unavailable" => Self::LocalModelUnavailable, "extraction_timeout" => Self::ExtractionTimeout, "summarizer_unavailable" => Self::SummarizerUnavailable, + "empty_input_refused" => Self::EmptyInputRefused, "transient" => Self::Transient, _ => return None, }) @@ -129,6 +138,7 @@ impl FailureCode { Self::LocalModelUnavailable => "memory.health.remediation.local_model_unavailable", Self::ExtractionTimeout => "memory.health.remediation.extraction_timeout", Self::SummarizerUnavailable => "memory.health.remediation.summarizer_unavailable", + Self::EmptyInputRefused => "memory.health.remediation.empty_input_refused", Self::Transient => "memory.health.remediation.transient", } } @@ -206,6 +216,20 @@ pub fn classify_embed_error(err: &anyhow::Error) -> PipelineFailure { pub fn classify_embed_error_str(msg: &str) -> PipelineFailure { let lower = msg.to_ascii_lowercase(); + // #13021: client-side refusal from the provider pre-flight guard fires + // *before* any HTTP round-trip, so it carries no `Embedding API error + // ()` shape. Without an explicit match it would fall through to + // `Transient` and the `reembed_backfill` worker would retry the same + // un-embeddable row forever (and eventually fail the whole job). + // Classify as unrecoverable per-row so the worker tombstones the chunk / + // summary instead. Both `OpenAiEmbedding::embed` and + // `OpenHumanCloudEmbedding::embed` use the literal phrase + // "refusing empty/whitespace". + if lower.contains("refusing empty/whitespace") { + return PipelineFailure::new(FailureCode::EmptyInputRefused) + .with_detail(truncate_detail(msg)); + } + // Dimension mismatch — the trait validator / CloudEmbedder rejects a // vector whose length isn't EMBEDDING_DIM. Check before status parsing: // it's a 2xx-but-wrong-shape case with no HTTP status to match. @@ -354,6 +378,7 @@ fn code_to_u8(code: FailureCode) -> u8 { FailureCode::ExtractionTimeout => 7, FailureCode::SummarizerUnavailable => 8, FailureCode::Transient => 9, + FailureCode::EmptyInputRefused => 10, } } @@ -368,6 +393,7 @@ fn u8_to_code(v: u8) -> Option { 7 => FailureCode::ExtractionTimeout, 8 => FailureCode::SummarizerUnavailable, 9 => FailureCode::Transient, + 10 => FailureCode::EmptyInputRefused, _ => return None, }) } @@ -451,7 +477,7 @@ pub fn current_degraded_state() -> DegradedState { mod tests { use super::*; - const ALL_CODES: [FailureCode; 9] = [ + const ALL_CODES: [FailureCode; 10] = [ FailureCode::BudgetExhausted, FailureCode::AuthMissing, FailureCode::AuthInvalid, @@ -460,6 +486,7 @@ mod tests { FailureCode::LocalModelUnavailable, FailureCode::ExtractionTimeout, FailureCode::SummarizerUnavailable, + FailureCode::EmptyInputRefused, FailureCode::Transient, ]; @@ -603,6 +630,46 @@ mod tests { assert!(f.is_unrecoverable()); } + /// #13021: the provider pre-flight bail wording from both OpenAI and the + /// cloud wrapper must classify as `EmptyInputRefused` (unrecoverable) so + /// `reembed_backfill` tombstones the offending row instead of retrying + /// the same blank input forever and eventually failing the job. + #[test] + fn classify_empty_input_refusal_as_unrecoverable() { + for msg in [ + "openai embed: refusing empty/whitespace input at index 0 of 1 (model=text-embedding-3-small)", + "cloud embed: refusing empty/whitespace input at index 2 of 5 (model=embedding-v1)", + ] { + let f = classify_embed_error_str(msg); + assert_eq!( + f.code, + FailureCode::EmptyInputRefused, + "expected EmptyInputRefused for {msg:?}" + ); + assert!( + f.is_unrecoverable(), + "EmptyInputRefused must be unrecoverable for {msg:?}" + ); + } + } + + /// The refusal must out-rank the dim-mismatch and budget rules even when + /// the wrapped error happens to contain those tokens — the refusal phrase + /// is the most specific signal and the only one that means "this row is + /// permanently un-embeddable", not "the provider is misbehaving". + #[test] + fn classify_empty_input_refusal_through_anyhow_context_chain() { + let base = anyhow::anyhow!( + "openai embed: refusing empty/whitespace input at index 0 of 1 (model=embedding-v1)" + ); + let wrapped = base + .context("embed summary during seal tree_id=t level=0") + .context("reembed_backfill chunk_id=c"); + let f = classify_embed_error(&wrapped); + assert_eq!(f.code, FailureCode::EmptyInputRefused); + assert!(f.is_unrecoverable()); + } + #[test] fn classify_5xx_is_transient() { let f = classify_embed_error_str("Embedding API error (503 Service Unavailable): retry"); diff --git a/src/openhuman/memory_tree/tree/bucket_seal.rs b/src/openhuman/memory_tree/tree/bucket_seal.rs index e18a78850..dfc717f65 100644 --- a/src/openhuman/memory_tree/tree/bucket_seal.rs +++ b/src/openhuman/memory_tree/tree/bucket_seal.rs @@ -412,8 +412,21 @@ pub(crate) async fn seal_one_level( target_level, token_budget: OUTPUT_TOKEN_BUDGET, }; + // #13021: treat a blank summary (LLM returned only whitespace, so + // `summarise()` succeeded with `content = ""`) the same as a hard error — + // fall back to the deterministic `fallback_summary` so we never persist a + // parent node with `content = ""` / `token_count = 0`. Without this, the + // child text is lost: the level buffer is cleared and the parent has no + // recoverable content for the next rollup or for retrieval. let output = match summarise(config, &inputs, &ctx).await { - Ok(o) => o, + Ok(o) if !o.content.trim().is_empty() => o, + Ok(_) => { + log::warn!( + "[memory_tree::seal] summarise returned blank for tree_id={} level={} — using fallback (#13021)", + ctx.tree_id, ctx.target_level, + ); + fallback_summary(&inputs, ctx.token_budget) + } Err(e) => { log::warn!( "[memory_tree::seal] summarise failed for tree_id={} level={}: {e:#} — using fallback", @@ -466,43 +479,59 @@ pub(crate) async fn seal_one_level( // (build_write_embedder returns None) rather than writing a fake all-zero // vector. The summary is sealed embedding-less (re-embeddable later) and // the semantic-recall degraded flag is already set with a typed cause. - let embedding: Option> = match build_write_embedder(config) - .context("build embedder during seal")? - { - None => { - log::warn!( - "[tree::bucket_seal] embeddings unavailable for tree_id={} level={}→{} \ + // + // #13021: also skip when `embed_input.trim().is_empty()`. `summarise()` + // returns `SummaryOutput::default()` (content = "") when every input is + // whitespace, and `fallback_summary` joins zero parts the same way. An + // empty `embed_input` is guaranteed to 400 from the upstream embedding + // API — short-circuit to the same "embedding-less, re-embeddable later" + // path as the no-provider case. + let embedding: Option> = if embed_input.trim().is_empty() { + log::warn!( + "[tree::bucket_seal] empty summary content for tree_id={} level={}→{} \ + — sealing without embedding (#13021)", + tree.id, + level, + target_level + ); + None + } else { + match build_write_embedder(config).context("build embedder during seal")? { + None => { + log::warn!( + "[tree::bucket_seal] embeddings unavailable for tree_id={} level={}→{} \ — sealing summary without embedding (semantic recall degraded)", - tree.id, - level, - target_level - ); - None - } - Some(embedder) => { - let v = match embedder.embed(&embed_input).await { - Ok(v) => v, - Err(e) => { - // #002: classify so the seal job fails fast on - // unrecoverable embed causes (budget/auth/dim) with a - // typed reason instead of retrying; original chain - // preserved as context. - let failure = crate::openhuman::memory_tree::health::classify_embed_error(&e); - return Err(anyhow::Error::new(failure).context(format!( - "embed summary during seal tree_id={} level={}: {e:#}", - tree.id, level - ))); - } - }; - // Dimension guard: reject wrong-dimensionality vectors before - // they reach the store — same contract as handle_extract's - // pack_checked. Without this a provider returning the wrong - // shape slips into the summary sidecar silently. - crate::openhuman::memory_tree::score::embed::pack_checked(&v).context(format!( - "seal embed dim check tree_id={} level={}", - tree.id, level - ))?; - log::debug!( + tree.id, + level, + target_level + ); + None + } + Some(embedder) => { + let v = match embedder.embed(&embed_input).await { + Ok(v) => v, + Err(e) => { + // #002: classify so the seal job fails fast on + // unrecoverable embed causes (budget/auth/dim) with a + // typed reason instead of retrying; original chain + // preserved as context. + let failure = + crate::openhuman::memory_tree::health::classify_embed_error(&e); + return Err(anyhow::Error::new(failure).context(format!( + "embed summary during seal tree_id={} level={}: {e:#}", + tree.id, level + ))); + } + }; + // Dimension guard: reject wrong-dimensionality vectors before + // they reach the store — same contract as handle_extract's + // pack_checked. Without this a provider returning the wrong + // shape slips into the summary sidecar silently. + crate::openhuman::memory_tree::score::embed::pack_checked(&v).context(format!( + "seal embed dim check tree_id={} level={}", + tree.id, level + ))?; + log::debug!( "[tree::bucket_seal] embedded summary tree_id={} level={}→{} bytes={} provider={}", tree.id, level, @@ -510,8 +539,9 @@ pub(crate) async fn seal_one_level( output.content.len(), embedder.name() ); - crate::openhuman::memory_tree::health::clear_semantic_recall_degraded(); - Some(v) + crate::openhuman::memory_tree::health::clear_semantic_recall_degraded(); + Some(v) + } } }; @@ -1216,8 +1246,18 @@ async fn seal_explicit_children( charged_amount_usd: None, } } else { + // #13021: blank summary (whitespace-only LLM output) → fallback. See + // the matching branch in `seal_one_level` for why we treat blank + // content as a hard fail rather than persisting `content = ""`. match summarise(config, &inputs, &ctx).await { - Ok(o) => o, + Ok(o) if !o.content.trim().is_empty() => o, + Ok(_) => { + log::warn!( + "[tree::bucket_seal] doc-subtree summarise returned blank tree_id={} doc_id_hash={} level={} — fallback (#13021)", + tree.id, doc_id.map(redact).unwrap_or_default(), level, + ); + fallback_summary(&inputs, ctx.token_budget) + } Err(e) => { log::warn!( "[tree::bucket_seal] doc-subtree summarise failed tree_id={} doc_id_hash={} level={}: {e:#} — fallback", @@ -1232,8 +1272,22 @@ async fn seal_explicit_children( // Embed before any write so a failure aborts cleanly — same contract as // seal_one_level. No-provider configs seal embedding-less. + // + // #13021: skip when the embed input is empty/whitespace. `summarise()` + // returns an empty content default when every input is whitespace, which + // would otherwise round-trip a guaranteed 400 from the upstream embedding + // API. The doc-subtree seal is sealed embedding-less, matching the + // no-provider branch. let embed_input = truncate_for_embed(&output.content, 1_000); - let embedding: Option> = + let embedding: Option> = if embed_input.trim().is_empty() { + log::warn!( + "[tree::bucket_seal] doc-subtree: empty summary content for tree_id={} level={} \ + — sealing without embedding (#13021)", + tree.id, + level + ); + None + } else { match build_write_embedder(config).context("build embedder during doc-subtree seal")? { None => None, Some(embedder) => { @@ -1250,7 +1304,8 @@ async fn seal_explicit_children( ))?; Some(v) } - }; + } + }; let now = Utc::now(); let summary_id = new_summary_id(target_level); diff --git a/src/openhuman/memory_tree/tree/bucket_seal_tests.rs b/src/openhuman/memory_tree/tree/bucket_seal_tests.rs index e8116900d..01b7ea264 100644 --- a/src/openhuman/memory_tree/tree/bucket_seal_tests.rs +++ b/src/openhuman/memory_tree/tree/bucket_seal_tests.rs @@ -1113,3 +1113,130 @@ async fn hydrate_summary_inputs_batch_preserves_order_and_skips_missing_ids() { assert_eq!(out[0].entities, vec!["entity:bob".to_string()]); assert_eq!(out[1].entities, vec!["entity:alice".to_string()]); } + +/// Regression for Sentry #13021 — when the LLM summary collapses to an +/// empty/whitespace string (e.g. the model returns just newlines, which +/// `summarise()` trims to ""), the seal MUST NOT persist a blank parent. +/// +/// Pre-fix, `truncate_for_embed("", 1000)` produced an empty string that +/// got forwarded to the embedding provider. Real providers (cloud / +/// OpenAI-compatible) 400 on that — `"input must be a non-empty string or +/// array of non-empty strings"` — and the failure was captured as a +/// recurring Sentry server fault even though the defect was on the client +/// side. +/// +/// Initial fix (provider guard) short-circuited the embed call but still +/// persisted `content = ""`, losing the child text from the next rollup / +/// retrieval layer. Final fix (this test): when `summarise()` returns +/// blank, fall back to `fallback_summary` — the deterministic concatenation +/// of the inputs — so the parent has recoverable text and the embedding +/// runs on real content. +#[tokio::test] +async fn whitespace_llm_summary_falls_back_to_deterministic_content() { + use crate::openhuman::memory_store::chunks::store::upsert_chunks; + use crate::openhuman::memory_store::chunks::types::{ + chunk_id, Chunk, Metadata, SourceKind, SourceRef, + }; + use chrono::TimeZone; + + let (_tmp, cfg) = test_config(); + let tree = get_or_create_source_tree(&cfg, "slack:#eng").unwrap(); + // The chat provider returns ONLY whitespace; `summarise()` trims to "" + // and `clamp_to_budget` keeps it empty → `output.content = ""`. + let provider: Arc = Arc::new(StaticChatProvider::new(" \n\t \n")); + + let ts = Utc.timestamp_millis_opt(1_700_000_000_000).unwrap(); + let mk_chunk = |seq: u32, tokens: u32| Chunk { + id: chunk_id(SourceKind::Chat, "slack:#eng", seq, "test-content"), + content: format!("non-empty leaf content {seq}"), + metadata: Metadata { + source_kind: SourceKind::Chat, + source_id: "slack:#eng".into(), + owner: "alice".into(), + timestamp: ts, + time_range: (ts, ts), + tags: vec![], + source_ref: Some(SourceRef::new("slack://x")), + path_scope: None, + }, + token_count: tokens, + seq_in_source: seq, + created_at: ts, + partial_message: false, + }; + let per_leaf = INPUT_TOKEN_BUDGET * 6 / 10; + let c1 = mk_chunk(0, per_leaf); + let c2 = mk_chunk(1, per_leaf); + upsert_chunks(&cfg, &[c1.clone(), c2.clone()]).unwrap(); + stage_test_chunks(&cfg, &[c1.clone(), c2.clone()]); + + let leaf1 = LeafRef { + chunk_id: c1.id.clone(), + token_count: per_leaf, + timestamp: ts, + content: c1.content.clone(), + entities: vec![], + topics: vec![], + score: 0.5, + }; + let leaf2 = LeafRef { + chunk_id: c2.id.clone(), + token_count: per_leaf, + timestamp: ts, + content: c2.content.clone(), + entities: vec![], + topics: vec![], + score: 0.5, + }; + + let _ = test_override::with_provider(Arc::clone(&provider), async { + append_leaf(&cfg, &tree, &leaf1, &LabelStrategy::Empty) + .await + .unwrap() + }) + .await; + let sealed = test_override::with_provider(Arc::clone(&provider), async { + append_leaf(&cfg, &tree, &leaf2, &LabelStrategy::Empty) + .await + .unwrap() + }) + .await; + + assert_eq!(sealed.len(), 1, "second append crosses budget — one seal"); + let summary = store::get_summary(&cfg, &sealed[0]).unwrap().unwrap(); + + // Content must be the deterministic fallback derived from the inputs, + // not the empty LLM output. `fallback_summary` joins each non-whitespace + // input with a `"— "` provenance prefix. + assert!( + !summary.content.trim().is_empty(), + "expected fallback content when LLM returned blank (#13021), got empty" + ); + assert!( + summary.content.contains("non-empty leaf content 0"), + "fallback must include leaf 0 content; got: {:?}", + summary.content + ); + assert!( + summary.content.contains("non-empty leaf content 1"), + "fallback must include leaf 1 content; got: {:?}", + summary.content + ); + + // Because the persisted content is now non-empty, the embed step runs. + // With `embeddings_provider = "none"` the test wires `InertEmbedder`, + // which returns a zero vector — its presence (not its value) is the + // signal that the new fallback path drove a real embed call. + // + // Read from the per-model sidecar (`mem_tree_summary_embeddings`); the + // legacy `mem_tree_summaries.embedding` column on `SummaryNode` is + // always written as `None` post-#1574 cutover, so checking + // `summary.embedding` alone would silently always pass. + let sidecar_embedding = + crate::openhuman::memory_store::trees::store::get_summary_embedding(&cfg, &sealed[0]) + .unwrap(); + assert!( + sidecar_embedding.is_some(), + "expected sidecar embedding for the fallback-filled summary (#13021)" + ); +}