diff --git a/navi/config.py b/navi/config.py index 88e7143..938a6fc 100644 --- a/navi/config.py +++ b/navi/config.py @@ -22,6 +22,11 @@ embedding_ollama_api_key: str = "" embedding_model: str = "nomic-embed-text:latest" embedding_dimensions: int = 768 + # Embedding models reject input past their context window with a 400, which + # costs the whole embedding — recall then falls back to ILIKE. Clip instead: + # a reaction-session prompt (a full event envelope inlined) failed 41 times. + # 0 disables the cap. + embedding_max_chars: int = 6000 openai_api_key: str = "" openai_model: str = "gpt-4" diff --git a/navi/llm/base.py b/navi/llm/base.py index f6faa6f..506b105 100644 --- a/navi/llm/base.py +++ b/navi/llm/base.py @@ -9,7 +9,7 @@ from datetime import datetime, timezone from typing import AsyncGenerator, Literal -from pydantic import BaseModel, Field +from pydantic import BaseModel, Field, field_validator class ToolCallRequest(BaseModel): @@ -67,6 +67,20 @@ # Marks messages that must survive compression verbatim (user corrections, exact tool output) is_compression_critical: bool = False + @field_validator("content", "thinking", mode="before") + @classmethod + def _strip_nul(cls, value): + """PostgreSQL text cannot hold a NUL byte, and one arriving in a tool + result (reading a binary file, a `cat` of one) made the whole + session_messages INSERT fail with CharacterNotInRepertoireError — the + turn died and the user lost it (three times on prod, 2026-10-07). Strip + at the model boundary so every writer downstream of Message is safe: + the session store, the kv store, memory extraction. + """ + if isinstance(value, str) and "\x00" in value: + return value.replace("\x00", "") + return value + class LLMResponse(BaseModel): """Non-streaming response from an LLM backend.""" diff --git a/navi/memory/_embeddings.py b/navi/memory/_embeddings.py index 8a0e14f..6dcaeff 100644 --- a/navi/memory/_embeddings.py +++ b/navi/memory/_embeddings.py @@ -16,6 +16,17 @@ _VECTOR_CUTOFF_DISTANCE = 0.3 +def _clip_for_embedding(text: str) -> str: + """Cap the input at the embedding model's window instead of letting the 400 + take the whole embedding with it. Recall then still works on the head of the + text — the alternative was ILIKE over everything (see settings.embedding_max_chars). + """ + limit = settings.embedding_max_chars + if limit and len(text) > limit: + return text[:limit] + return text + + def _vector_to_str(vec: list[float]) -> str | None: """Serialize a vector to the PostgreSQL vector literal format. @@ -57,7 +68,7 @@ return None try: vectors = await self._embedding_backend.embed( - texts=[text], + texts=[_clip_for_embedding(text)], model=settings.embedding_model, ) if vectors and vectors[0]: @@ -71,7 +82,7 @@ return [None] * len(texts) try: vectors = await self._embedding_backend.embed( - texts=texts, + texts=[_clip_for_embedding(t) for t in texts], model=settings.embedding_model, ) return [v if v else None for v in vectors] diff --git a/navi/memory/_facts.py b/navi/memory/_facts.py index ee85b67..57ece27 100644 --- a/navi/memory/_facts.py +++ b/navi/memory/_facts.py @@ -74,7 +74,7 @@ """INSERT INTO memory_facts (id, user_id, category, key, value, created_at, updated_at, source_session_id, source, confidence, expires_at, source_context) - VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12) ON CONFLICT(user_id, category, key) DO UPDATE SET value = EXCLUDED.value, updated_at = EXCLUDED.updated_at, diff --git a/tests/unit/llm/test_message.py b/tests/unit/llm/test_message.py new file mode 100644 index 0000000..a42d376 --- /dev/null +++ b/tests/unit/llm/test_message.py @@ -0,0 +1,24 @@ +"""Message is the boundary every stored text passes through.""" + +from navi.llm.base import Message + + +class TestNulBytes: + """PostgreSQL text cannot hold a NUL byte, and one arriving in a tool result + made the whole session_messages INSERT fail with CharacterNotInRepertoireError + — the turn died and the user lost it (three times on prod, 2026-10-07). The + strip lives on Message so it covers every writer downstream of it. + """ + + def test_content_loses_nul_bytes(self): + assert Message(role="tool", content="bi\x00nary").content == "binary" + + def test_thinking_loses_nul_bytes_too(self): + assert Message(role="assistant", thinking="th\x00ink").thinking == "think" + + def test_clean_text_is_untouched(self): + text = "привет, мир — 100% ok" + assert Message(role="user", content=text).content == text + + def test_absent_content_stays_none(self): + assert Message(role="assistant").content is None diff --git a/tests/unit/memory/test_store.py b/tests/unit/memory/test_store.py index 3982f1b..b5f53ac 100644 --- a/tests/unit/memory/test_store.py +++ b/tests/unit/memory/test_store.py @@ -1,7 +1,10 @@ """Unit tests for MemoryStore (mocked asyncpg).""" +import re + import pytest +from navi.config import settings from tests.conftest_factory import FakeConnection, FakeRecord, FakePool, make_store_with_pool @@ -15,14 +18,63 @@ assert "memory_facts" in conn.calls[0][1] async def test_without_embedding(self): + """The no-embedding branch bound one placeholder more than it had columns + (13 vs 12), so every fact written while the embedding backend was down + died on PostgresSyntaxError and the extraction was lost — 47 embed + failures in three days on prod, most of them reaching this branch. + """ conn = FakeConnection() - conn.enqueue("INSERT 0 1") # no embedding column branch + conn.enqueue("INSERT 0 1") store = make_store_with_pool(conn) store._embedding_backend = None + await store.upsert_fact(category="profile", key="name", value="Eugene") - call = conn.calls[0] - # Should use the branch without embedding - assert "embedding" not in call[1] or "$8" in call[1] # no vector parameter + + _, query, args = conn.calls[0] + columns = query.split("(", 1)[1].split(")", 1)[0].split(",") + highest_placeholder = max(int(n) for n in re.findall(r"\$(\d+)", query)) + assert "embedding" not in query + assert highest_placeholder == len(columns) == len(args) + + +class TestEmbeddingInputLength: + """An input past the model's window came back as a 400 and took the whole + embedding with it — 41 reaction-session prompts did that on prod, each one + dropping recall back to ILIKE. Clip the head instead of losing the vector. + """ + + class _RecordingBackend: + def __init__(self): + self.texts: list[str] = [] + + async def embed(self, texts, model=None): + self.texts = list(texts) + return [[0.1] * 768 for _ in texts] + + def _store(self): + store = make_store_with_pool(FakeConnection()) + store._pgvector_checked = True + store._pgvector_available = True + store._embedding_backend = self._RecordingBackend() + return store + + async def test_single_text_is_clipped_before_the_backend_sees_it(self): + store = self._store() + over = settings.embedding_max_chars + 500 + + await store._generate_embedding("x" * over) + + assert len(store._embedding_backend.texts[0]) == settings.embedding_max_chars + + async def test_batch_clips_each_text_and_leaves_short_ones_alone(self): + store = self._store() + + await store._generate_embeddings(["y" * (settings.embedding_max_chars * 2), "short"]) + + assert [len(t) for t in store._embedding_backend.texts] == [ + settings.embedding_max_chars, + len("short"), + ] class TestSearchFacts: