Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,20 @@ EMBEDDINGS_PROVIDER=disabled

# Embedding model (used when provider is openai or local).
# OpenAI: text-embedding-3-small
# Local: a sentence-transformers model name (requires raglogs[local-embeddings])
# Local: a sentence-transformers model name (requires raglogs[local-embeddings]).
# IMPORTANT: the stored embedding column is Vector(1536), so a local
# model MUST emit exactly 1536 dims. The popular models do NOT
# (all-MiniLM-L6-v2 = 384, all-mpnet-base-v2 = 768) and will be
# rejected loudly at startup / before ingest rather than silently
# producing no embeddings. Use OpenAI or disabled unless you have a
# 1536-dim local model.
EMBEDDINGS_MODEL=text-embedding-3-small

# Embedding vector dimensions — must match the model output AND the stored
# log_embeddings column (Vector(1536)). Persist and semantic ask skip when this
# is not 1536. text-embedding-3-small = 1536, text-embedding-3-large = 3072
# log_embeddings column (Vector(1536)). Any value other than 1536 with a
# non-disabled provider is a hard configuration error (nothing could be
# persisted): startup and `ingest --with-embeddings` fail with an actionable
# message. text-embedding-3-small = 1536, text-embedding-3-large = 3072.
EMBEDDINGS_DIMENSIONS=1536

# Merge fingerprint clusters whose representative messages have cosine
Expand Down
6 changes: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,7 @@ Populate vectors at ingest time:
raglogs ingest ./logs --with-embeddings
```

Vectors are stored in `log_embeddings` (1536-d). If `EMBEDDINGS_DIMENSIONS` is not 1536, persist is skipped so the schema stays valid. Provider failures during ingest are skipped; the log lines still land.
Vectors are stored in `log_embeddings` (1536-d). Because that column width is fixed, a non-disabled provider with `EMBEDDINGS_DIMENSIONS ≠ 1536` — or a `local` model whose native width isn't 1536 — is a hard configuration error: `ingest --with-embeddings` and API startup fail with an actionable message instead of silently persisting nothing. Transient provider failures *during* ingest are still skipped fail-open; the log lines land regardless.

**Example output**

Expand Down Expand Up @@ -702,7 +702,7 @@ All settings are read from `.env`, environment variables, or CLI flags. Priority
| `ANTHROPIC_BASE_URL` | `https://api.anthropic.com` | Anthropic Messages API host |
| `EMBEDDINGS_PROVIDER` | `disabled` | `disabled`, `openai`, `local`. Cluster merge, semantic `ask`, and `/similar` ANN skip when `disabled` |
| `EMBEDDINGS_MODEL` | `text-embedding-3-small` | Embeddings model name |
| `EMBEDDINGS_DIMENSIONS` | `1536` | Vector size passed to the OpenAI embeddings API. Persist/ask skip unless this is 1536 (stored column width) |
| `EMBEDDINGS_DIMENSIONS` | `1536` | Vector size. Must be 1536 (the stored column width) for any non-disabled provider; any other value is a hard config error surfaced at startup / before ingest |
| `CLUSTER_MERGE_SIMILARITY_THRESHOLD` | `0.92` | Cosine similarity at or above which fingerprint clusters merge. High on purpose so distinct errors stay separate |
| `CLUSTER_MERGE_MIN_COUNT` | `1` | Minimum `count` for a cluster to participate in a merge |
| `ASK_SEMANTIC_TOP_K` | `100` | Max log lines returned by semantic `ask` |
Expand Down Expand Up @@ -913,7 +913,7 @@ Fingerprinting can still split one incident across multiple clusters when wordin
- **Cluster template persist.** After clustering, raglogs upserts each cluster's representative template into `cluster_embeddings` keyed by `(scope, fingerprint)` when the embeddings provider is available. Similar-incident search queries those rows (not raw `log_embeddings`). Skip happens automatically when `EMBEDDINGS_PROVIDER=disabled`.
- **Ask vs merge vs similar.** Semantic `ask` uses the *stored* `log_embeddings` table (populated by `raglogs ingest --with-embeddings`) and `ASK_SEMANTIC_MIN_SIMILARITY` (default **0.75**). Cluster merge still uses its own in-memory pass and threshold. Similar-incident search (`POST /v1/query/similar`) uses the `cluster_embeddings` table (upserted at analysis time when an embeddings provider is available) and `SIMILAR_SEMANTIC_MIN_SIMILARITY` (default **0.80**). Compare still applies its own heuristic collapse for webhook retries / queue growth after clustering.

Local embeddings require the optional extra: `pip install 'raglogs[local-embeddings]'` (`sentence-transformers`). If that import fails, merge is skipped.
Local embeddings require the optional extra: `pip install 'raglogs[local-embeddings]'` (`sentence-transformers`) **and** a model that emits exactly 1536 dims to match the stored column. The common models don't (`all-MiniLM-L6-v2` = 384, `all-mpnet-base-v2` = 768), so `EMBEDDINGS_PROVIDER=local` with one of them fails loudly at startup / before ingest rather than silently producing no embeddings. On the analysis-time merge hot path a missing extra or bad config degrades to skip (logged at `error`), never a crash.

### Baseline comparison

Expand Down
12 changes: 12 additions & 0 deletions src/api/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,18 @@ async def lifespan(app: FastAPI) -> AsyncIterator[None]:
setup_observability(log_format=settings.log_format)
warn_if_insecure_bind(settings.api_bind_host, settings)

from src.core.embeddings.provider import (
EmbeddingsConfigError,
validate_embeddings_config,
)

try:
validate_embeddings_config(settings)
except EmbeddingsConfigError as exc:

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MAJOR — declared behavior vs actual: API startup does not fail here, it logs and continues.

The README (Vectors are stored…API startup fail with an actionable message), .env.example (startup and \ingest --with-embeddings` fail with an actionable message), and this PR's own acceptance criterion (fails loudly at startup/ingest) all state that a non-persistable embeddings config makes **API startup fail**. But this handler catches EmbeddingsConfigErrorand only emits oneerrorlog, thenyield`s — the app boots and serves normally.

CONFIRMED asymmetry with the ingest path, which genuinely fails: src/cli/commands/ingest.py:74 does raise typer.Exit(1) on the same error. So "fails loudly at startup/ingest" is true for ingest, false for the API.

Operational consequence: an operator who deploys the API with EMBEDDINGS_PROVIDER=local + a 384/768-dim model (the exact case #84 is about) gets a green, healthy-looking server that will never persist a vector and silently keyword-falls-back on /v1/query/similar and semantic ask — with only a single startup log line, which is easy to miss. That's close to the silent no-op the issue set out to eliminate.

Two clean resolutions — pick one:

  1. Match the docs: re-raise after logging (e.g. raise / raise RuntimeError(...)) so the process refuses to boot on a broken config. Note this couples the whole API's availability to an optional feature, which may be too aggressive during an incident.
  2. Match the code (my preference): keep logging-and-continue, but reword the README + .env.example to say the API logs the misconfig at error on startup and continues serving non-embedding features, and only ingest --with-embeddings hard-exits. That keeps explain/timeline/compare available while still being non-silent.

Either way, docs and code should agree. Right now they don't.

import structlog

structlog.get_logger().error("embeddings_config_invalid", reason=str(exc))

if not check_connection():
import structlog
log = structlog.get_logger()
Expand Down
13 changes: 12 additions & 1 deletion src/cli/commands/ingest.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,23 @@ def ingest_cmd(

if with_embeddings:
from src.config import get_settings
from src.core.embeddings.provider import (
EmbeddingsConfigError,
validate_embeddings_config,
)

if get_settings().embeddings_provider == "disabled":
settings = get_settings()
if settings.embeddings_provider == "disabled":
console.print(
"[yellow]Warning:[/yellow] --with-embeddings requested but "
"EMBEDDINGS_PROVIDER=disabled; ingesting without embeddings."
)
else:
try:
validate_embeddings_config(settings)
except EmbeddingsConfigError as e:
console.print(f"[red]Error:[/red] {e}")
raise typer.Exit(1)

with Progress(
SpinnerColumn(),
Expand Down
102 changes: 94 additions & 8 deletions src/core/embeddings/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,31 @@
from typing import Protocol, runtime_checkable

import httpx
import structlog
from tenacity import retry, stop_after_attempt, wait_exponential

from src.config import get_settings
from src.config.settings import Settings

log = structlog.get_logger()

# OpenAI allows large batches; keep requests modest for local-compatible APIs.
_OPENAI_BATCH_SIZE = 96

# The persisted pgvector column is fixed at this width (migration 0001,
# ``log_embeddings.embedding`` / ``cluster_embeddings.embedding``). Any provider
# that emits a different dimension cannot be stored, so we refuse it loudly
# rather than silently dropping every vector.
STORED_EMBEDDING_DIMS = 1536


class EmbeddingsConfigError(RuntimeError):
"""A configured embeddings provider cannot possibly persist vectors.

Raised loudly (at startup / before ingest) instead of letting the storage
layer silently skip every write — see issue #84.
"""


@runtime_checkable
class EmbeddingsProvider(Protocol):
Expand Down Expand Up @@ -86,9 +103,16 @@ def _embed_batch(self, texts: list[str]) -> list[list[float]]:


class LocalEmbeddingsProvider:
"""Optional local encoder. Construction raises ImportError if the extra is missing."""
"""Optional local encoder.

Construction raises ``ImportError`` if the extra is missing and
``EmbeddingsConfigError`` if the model's native embedding width does not
match the stored ``Vector`` column — the common case, since the popular
local models (``all-MiniLM-L6-v2`` = 384, ``all-mpnet-base-v2`` = 768) do
not emit 1536 dims and would otherwise be dropped silently at persist time.
"""

def __init__(self, model: str) -> None:
def __init__(self, model: str, expected_dimensions: int = STORED_EMBEDDING_DIMS) -> None:
try:
from sentence_transformers import SentenceTransformer
except ImportError as exc:
Expand All @@ -97,6 +121,16 @@ def __init__(self, model: str) -> None:
"Install with: pip install 'raglogs[local-embeddings]'"
) from exc
self._model = SentenceTransformer(model)
actual = self._model.get_sentence_embedding_dimension()
if actual != expected_dimensions:
raise EmbeddingsConfigError(
f"EMBEDDINGS_PROVIDER=local model '{model}' emits {actual}-dim vectors, "
f"but the stored embedding column is Vector({expected_dimensions}). "
f"Local embeddings only work with a model that outputs exactly "
f"{expected_dimensions} dims. Choose such a model, or use "
f"EMBEDDINGS_PROVIDER=openai (text-embedding-3-small = 1536), or "
f"EMBEDDINGS_PROVIDER=disabled."
)

def is_available(self) -> bool:
return True
Expand All @@ -108,8 +142,15 @@ def embed_texts(self, texts: list[str]) -> list[list[float]]:
return [vector.tolist() for vector in vectors]


def get_embeddings_provider(settings: Settings | None = None) -> EmbeddingsProvider:
"""Factory: build the configured embeddings provider. Fail-open to disabled."""
def build_embeddings_provider(settings: Settings | None = None) -> EmbeddingsProvider:
"""Strict factory: build the configured provider or raise on a broken config.

Raises ``ImportError`` (local extra missing) or ``EmbeddingsConfigError``
(local model dimension mismatch). ``openai`` with no key returns the
disabled provider — that is "not configured yet", not a broken config.
Use :func:`validate_embeddings_config` at startup and
:func:`get_embeddings_provider` on the hot path.
"""
if settings is None:
settings = get_settings()

Expand All @@ -124,9 +165,54 @@ def get_embeddings_provider(settings: Settings | None = None) -> EmbeddingsProvi
)

if settings.embeddings_provider == "local":
try:
return LocalEmbeddingsProvider(model=settings.embeddings_model)
except ImportError:
return DisabledEmbeddingsProvider()
return LocalEmbeddingsProvider(
model=settings.embeddings_model,
expected_dimensions=STORED_EMBEDDING_DIMS,
)

return DisabledEmbeddingsProvider()


def get_embeddings_provider(settings: Settings | None = None) -> EmbeddingsProvider:
"""Hot-path factory: fail-open to disabled so a misconfig never crashes ingest.

A broken configuration is logged at ``error`` (never silently swallowed) and
degrades to the disabled provider. Startup calls
:func:`validate_embeddings_config` first, so in practice this path only sees
a healthy config or an already-surfaced one.
"""
try:
return build_embeddings_provider(settings)
except (ImportError, EmbeddingsConfigError) as exc:
log.error("embeddings_provider_disabled", reason=str(exc))
return DisabledEmbeddingsProvider()


def validate_embeddings_config(settings: Settings | None = None) -> None:
"""Fail loudly if the embeddings configuration can never persist a vector.

Call at startup / before ingest. Raises :class:`EmbeddingsConfigError` when
a non-disabled provider is configured with a dimension the stored column
cannot hold, or (for ``local``) a model whose native width does not match.
``disabled`` is always valid. See issue #84.
"""
if settings is None:
settings = get_settings()

if settings.embeddings_provider == "disabled":
return

if settings.embeddings_dimensions != STORED_EMBEDDING_DIMS:
raise EmbeddingsConfigError(
f"EMBEDDINGS_DIMENSIONS={settings.embeddings_dimensions} but the stored "
f"embedding column is Vector({STORED_EMBEDDING_DIMS}). No embeddings can be "
f"persisted with this setting. Set EMBEDDINGS_DIMENSIONS={STORED_EMBEDDING_DIMS} "
f"(e.g. text-embedding-3-small) or EMBEDDINGS_PROVIDER=disabled."
)

if settings.embeddings_provider == "local":
# Constructing loads the model and verifies its real output dimension.
try:
build_embeddings_provider(settings)
except ImportError as exc:
raise EmbeddingsConfigError(str(exc)) from exc
10 changes: 7 additions & 3 deletions src/core/embeddings/store.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,11 @@

from src.config import get_settings
from src.config.settings import Settings
from src.core.embeddings.provider import EmbeddingsProvider, get_embeddings_provider
from src.core.embeddings.provider import (
STORED_EMBEDDING_DIMS,
EmbeddingsProvider,
get_embeddings_provider,
)
from src.db.models import (
CLUSTER_EMBEDDING_UNIQUE,
ClusterEmbedding,
Expand All @@ -32,8 +36,8 @@

log = structlog.get_logger()

# Must match ``log_embeddings.embedding Vector(1536)`` in the applied schema.
STORED_EMBEDDING_DIMS = 1536
# ``STORED_EMBEDDING_DIMS`` is defined in provider.py (the canonical source) and
# re-exported here for the existing call sites below.


def ingest_embeddings_provider(
Expand Down
91 changes: 90 additions & 1 deletion tests/unit/test_embeddings_provider.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,46 @@
"""Unit tests for the embeddings provider factory. No live API calls."""

import sys
import types
from contextlib import contextmanager
from unittest.mock import MagicMock, patch

import pytest

from src.config.settings import Settings
from src.core.embeddings.provider import (
STORED_EMBEDDING_DIMS,
DisabledEmbeddingsProvider,
EmbeddingsConfigError,
LocalEmbeddingsProvider,
OpenAIEmbeddingsProvider,
get_embeddings_provider,
validate_embeddings_config,
)


@contextmanager
def _fake_sentence_transformers(dim: int):
"""Patch ``sentence_transformers`` with a stub model of a given output dim."""
module = types.ModuleType("sentence_transformers")

class _FakeSentenceTransformer:
def __init__(self, model: str) -> None:
self._model = model

def get_sentence_embedding_dimension(self) -> int:
return dim

def encode(self, texts, convert_to_numpy=True):
import numpy as np

return np.zeros((len(texts), dim))

module.SentenceTransformer = _FakeSentenceTransformer
with patch.dict(sys.modules, {"sentence_transformers": module}):
yield


def test_disabled_is_unavailable() -> None:
provider = DisabledEmbeddingsProvider()
assert provider.is_available() is False
Expand Down Expand Up @@ -48,7 +78,7 @@ def test_factory_local_import_error_is_disabled() -> None:
settings = Settings(_env_file=None, embeddings_provider="local")

class Boom:
def __init__(self, model: str) -> None:
def __init__(self, model: str, expected_dimensions: int = 1536) -> None:
raise ImportError("sentence-transformers missing")

with patch("src.core.embeddings.provider.LocalEmbeddingsProvider", Boom):
Expand All @@ -69,6 +99,65 @@ def test_local_provider_missing_extra() -> None:
assert raised is True


def test_local_provider_dimension_mismatch_raises() -> None:
# 384-dim model (all-MiniLM-L6-v2) can't fill the Vector(1536) column.
with _fake_sentence_transformers(dim=384):
with pytest.raises(EmbeddingsConfigError) as exc:
LocalEmbeddingsProvider(model="all-MiniLM-L6-v2")
msg = str(exc.value)
assert "384" in msg and str(STORED_EMBEDDING_DIMS) in msg


def test_local_provider_matching_dimension_constructs() -> None:
with _fake_sentence_transformers(dim=STORED_EMBEDDING_DIMS):
provider = LocalEmbeddingsProvider(model="some-1536-model")
assert provider.is_available() is True


def test_factory_local_dimension_mismatch_fails_open_to_disabled() -> None:
# The hot-path factory never raises: it degrades to disabled (logged).
settings = Settings(_env_file=None, embeddings_provider="local")
with _fake_sentence_transformers(dim=768):
provider = get_embeddings_provider(settings)
assert isinstance(provider, DisabledEmbeddingsProvider)


def test_validate_disabled_is_ok() -> None:
validate_embeddings_config(Settings(_env_file=None)) # no raise


def test_validate_rejects_non_1536_dimensions() -> None:
settings = Settings(
_env_file=None,
embeddings_provider="openai",
openai_api_key="sk-test",
embeddings_dimensions=384,
)
with pytest.raises(EmbeddingsConfigError) as exc:
validate_embeddings_config(settings)
assert "384" in str(exc.value)


def test_validate_local_dimension_mismatch_raises() -> None:
settings = Settings(
_env_file=None,
embeddings_provider="local",
embeddings_model="all-mpnet-base-v2",
embeddings_dimensions=1536,
)
with _fake_sentence_transformers(dim=768):
with pytest.raises(EmbeddingsConfigError):
validate_embeddings_config(settings)


def test_validate_local_missing_extra_raises() -> None:
settings = Settings(_env_file=None, embeddings_provider="local")
with patch.dict(sys.modules, {"sentence_transformers": None}):
with pytest.raises(EmbeddingsConfigError) as exc:
validate_embeddings_config(settings)
assert "sentence-transformers" in str(exc.value)


def test_openai_embed_texts_restores_index_order() -> None:
fake_response = MagicMock()
fake_response.json.return_value = {
Expand Down