diff --git a/.env.template b/.env.template index 2b11a80c..7d169d4d 100644 --- a/.env.template +++ b/.env.template @@ -278,8 +278,8 @@ VECTOR_STORE_MIGRATED=false # - Messages: {NAMESPACE}.msg.{hash(workspace)} # VECTOR_STORE_NAMESPACE=honcho # Inherits from NAMESPACE if not set -# Embedding dimensions (default: 1536 for OpenAI text-embedding-3-small) -# VECTOR_STORE_DIMENSIONS=1536 +# Embedding dimensions are configured via EMBEDDING_VECTOR_DIMENSIONS (see top +# of this file). VECTOR_STORE_DIMENSIONS is deprecated and ignored. # Turbopuffer-specific settings (required if TYPE is "turbopuffer") # VECTOR_STORE_TURBOPUFFER_API_KEY=your-turbopuffer-api-key diff --git a/config.toml.example b/config.toml.example index 4c90e27c..8bf30878 100644 --- a/config.toml.example +++ b/config.toml.example @@ -254,9 +254,7 @@ TYPE = "pgvector" # Migration flag: set to true when migration from pgvector is complete MIGRATED = false NAMESPACE = "honcho" -# This should match embedding.vector_dimensions. pgvector and dual-write mode -# currently still require 1536 until a schema migration lands. -DIMENSIONS = 1536 +# DIMENSIONS is deprecated; embedding.vector_dimensions is authoritative. # TURBOPUFFER_API_KEY = "your-turbopuffer-api-key" # TURBOPUFFER_REGION = "us-east-1" LANCEDB_PATH = "./lancedb_data" diff --git a/docs/v3/contributing/configuration.mdx b/docs/v3/contributing/configuration.mdx index 8ceb3d83..9aacbb02 100644 --- a/docs/v3/contributing/configuration.mdx +++ b/docs/v3/contributing/configuration.mdx @@ -220,9 +220,9 @@ EMBEDDING_MODEL_CONFIG__OVERRIDES__API_KEY_ENV=EMBEDDING_CUSTOM_API_KEY Forwarding `dimensions=` to OpenAI-compatible providers is controlled by `EMBEDDING_MODEL_CONFIG__DIMENSIONS_MODE`: -- `auto` (default): forwards `dimensions=` when you have explicitly set `EMBEDDING_VECTOR_DIMENSIONS` to a non-default value, and the configured model is not on the known-rejecting list (currently `text-embedding-ada-002`). Deployments that stay on the default keep their existing behavior. -- `always`: always forward. Use for OpenAI-compatible self-hosted providers that require it, and for same-as-default truncation (e.g. `text-embedding-3-large` truncated to 1536 — `auto` would skip the parameter because 1536 is the default). -- `never`: never forward. Explicit opt-out for providers that reject the parameter. +- `auto` (default): forwards `dimensions=` when **the operator has explicitly set `EMBEDDING_VECTOR_DIMENSIONS`** — provenance, not value — and the configured model is not on the known-rejecting list (currently `text-embedding-ada-002`). Explicit `EMBEDDING_VECTOR_DIMENSIONS=1536` *does* trigger the forward; this is how `text-embedding-3-large` truncation to 1536 is expressed. Deployments that leave the setting unset get their existing behavior (`dimensions=` is not forwarded). +- `always`: always forward, regardless of whether `EMBEDDING_VECTOR_DIMENSIONS` was set. Use for OpenAI-compatible self-hosted providers that require it. Do not pick `always` *just* for same-as-default truncation — `auto` handles that case correctly as long as you set `EMBEDDING_VECTOR_DIMENSIONS=1536` explicitly in your environment. `always` is the right answer when your config layer might strip explicit "default-valued" envs, or when you want defense-in-depth. +- `never`: never forward. Explicit opt-out for providers that reject the parameter (e.g. `text-embedding-ada-002` if it slips past the known-rejecting allowlist). #### Bootstrapping non-default dimensions @@ -498,7 +498,8 @@ WEBHOOK_MAX_WORKSPACE_LIMIT=10 VECTOR_STORE_TYPE=pgvector # Options: pgvector, turbopuffer, lancedb VECTOR_STORE_MIGRATED=false VECTOR_STORE_NAMESPACE=honcho -VECTOR_STORE_DIMENSIONS=1536 +# Embedding dim is configured via EMBEDDING_VECTOR_DIMENSIONS — see the +# Embedding Configuration section. VECTOR_STORE_DIMENSIONS is deprecated. # Turbopuffer-specific VECTOR_STORE_TURBOPUFFER_API_KEY=your-turbopuffer-api-key @@ -676,4 +677,4 @@ uv run alembic revision --autogenerate -m "Description" # Create new migration 5. **Dialectic level issues** — Unset level fields inherit from the built-in defaults. For Anthropic, `THINKING_BUDGET_TOKENS` must be >= 1024 when enabled. For providers without budgeted thinking, omit it or set it to `0`. `MAX_OUTPUT_TOKENS` must exceed `THINKING_BUDGET_TOKENS`. -6. **Vector store issues** — For Turbopuffer, set the API key. Check `VECTOR_STORE_DIMENSIONS` matches your embedding model. +6. **Vector store issues** — For Turbopuffer, set the API key. Check that `EMBEDDING_VECTOR_DIMENSIONS` matches your embedding model — the startup validator will refuse to boot on a mismatch. diff --git a/scripts/configure_embeddings.py b/scripts/configure_embeddings.py index bde9a373..389876a4 100644 --- a/scripts/configure_embeddings.py +++ b/scripts/configure_embeddings.py @@ -37,11 +37,13 @@ _PROJECT_ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), "..")) if _PROJECT_ROOT not in sys.path: sys.path.insert(0, _PROJECT_ROOT) -from sqlalchemy import text # noqa: E402 +from sqlalchemy import select, text # noqa: E402 from sqlalchemy.ext.asyncio import AsyncConnection, AsyncEngine # noqa: E402 from src.config import settings # noqa: E402 from src.db import engine # noqa: E402 +from src.models import Collection, Workspace # noqa: E402 +from src.vector_store import VectorStore # noqa: E402 logger = logging.getLogger(__name__) @@ -66,7 +68,7 @@ class _NamespaceRecord: """A single row in the --report output.""" namespace: str - status: str # one of: "ok", "missing", "mismatch", "unknown" + status: str # one of: "ok", "missing", "mismatch" actual_dim: int | None target_dim: int @@ -229,17 +231,23 @@ async def _apply_pgvector_alter(engine: AsyncEngine, plan: _PgvectorPlan) -> Non async def _enumerate_workspaces(conn: AsyncConnection) -> list[str]: - result = await conn.execute(text("SELECT name FROM workspaces ORDER BY created_at")) - return [row.name for row in result] + """All workspace names, ordered by creation. Uses the ORM so + ``Base.metadata.schema`` (configured from ``DB.SCHEMA``) is honored — + non-public schema deployments must not sample the wrong table.""" + stmt = select(Workspace.name).order_by(Workspace.created_at) + result = await conn.execute(stmt) + return [row[0] for row in result] async def _enumerate_collections( conn: AsyncConnection, ) -> list[tuple[str, str, str]]: - result = await conn.execute( - text("SELECT workspace_name, observer, observed FROM collections") - ) - return [(row.workspace_name, row.observer, row.observed) for row in result] + """Every (workspace_name, observer, observed) triple that has a row in + the collections table — these are the document namespaces that could + exist in an external store.""" + stmt = select(Collection.workspace_name, Collection.observer, Collection.observed) + result = await conn.execute(stmt) + return [(row[0], row[1], row[2]) for row in result] async def _build_external_namespace_inventory( @@ -274,15 +282,10 @@ async def _build_external_namespace_inventory( return pairs -async def _probe_namespace_dim(store: object, namespace: str) -> int | None: - """Best-effort: return the namespace's declared dim if introspectable. - - Returns ``None`` if the SDK does not expose a uniform dim accessor for - the configured store. Future work can specialize per store; today the - pgvector validator is the load-bearing dim safety. - """ - _ = (store, namespace) - return None +async def _probe_namespace_dim(store: VectorStore, namespace: str) -> int | None: + """Return the namespace's declared dim, or ``None`` if the namespace + does not exist yet. Delegates to the store-specific probe.""" + return await store.probe_namespace_dim(namespace) async def _emit_report(engine: AsyncEngine, target_dim: int) -> int: @@ -304,12 +307,16 @@ async def _emit_report(engine: AsyncEngine, target_dim: int) -> int: from src.vector_store import get_external_vector_store store = get_external_vector_store() + if store is None: + print("no external store configured; nothing to report") + return 0 records: list[_NamespaceRecord] = [] for _ns_type, namespace in inventory: - actual = await _probe_namespace_dim(store, namespace) if store else None + actual = await _probe_namespace_dim(store, namespace) if actual is None: - status = "unknown" + # Namespace has not been written to yet (lazy-create model). + status = "missing" elif actual == target_dim: status = "ok" else: diff --git a/src/startup/embedding_validator.py b/src/startup/embedding_validator.py index 82c6f53e..ca996c16 100644 --- a/src/startup/embedding_validator.py +++ b/src/startup/embedding_validator.py @@ -16,11 +16,13 @@ from __future__ import annotations import asyncio import logging -from sqlalchemy import text +from sqlalchemy import select, text from sqlalchemy.exc import SQLAlchemyError from sqlalchemy.ext.asyncio import AsyncEngine from src.config import AppSettings, settings +from src.models import Workspace +from src.vector_store import VectorStore logger = logging.getLogger(__name__) @@ -195,25 +197,23 @@ async def _sample_external_namespaces(engine: AsyncEngine, *, target_dim: int) - async def _sample_workspace_names(engine: AsyncEngine, limit: int) -> list[str]: - """Pull up to ``limit`` workspace names ordered by creation time.""" - query = text("SELECT name FROM workspaces ORDER BY created_at DESC LIMIT :limit") - async with engine.connect() as conn: - result = await conn.execute(query, {"limit": limit}) - return [row.name for row in result] + """Pull up to ``limit`` workspace names ordered by creation time. - -async def _probe_namespace_dim(store: object, namespace: str) -> int | None: - """Best-effort: return the namespace's declared dim if introspectable. - - Returns ``None`` if the namespace does not exist or the SDK does not - expose dim metadata for the configured store. Callers must treat - ``None`` as "do not flag a mismatch" — full enumeration is delegated to - the ``configure_embeddings --report`` path. + Uses the ORM ``Workspace`` model so ``Base.metadata.schema`` (configured + from ``settings.DB.SCHEMA`` in ``src/db.py``) is honored automatically — + a non-public schema deployment must not silently sample the wrong table. """ - # The base ``VectorStore`` does not currently expose introspection of an - # individual namespace's dim. Until a concrete probe is added per store, - # treat all sampled namespaces as opaque. The pgvector check above is the - # load-bearing safety; first-write per workspace inherits its correctness - # from the embedding client honoring settings.EMBEDDING.VECTOR_DIMENSIONS. - _ = (store, namespace) - return None + stmt = select(Workspace.name).order_by(Workspace.created_at.desc()).limit(limit) + async with engine.connect() as conn: + result = await conn.execute(stmt) + return [row[0] for row in result] + + +async def _probe_namespace_dim(store: VectorStore, namespace: str) -> int | None: + """Return the namespace's declared dim, or ``None`` if not present. + + Delegates to the store's own ``probe_namespace_dim`` implementation + (lancedb opens the table, turbopuffer reads the schema). ``None`` means + "lazy-create namespace, nothing to validate against." + """ + return await store.probe_namespace_dim(namespace) diff --git a/src/vector_store/__init__.py b/src/vector_store/__init__.py index 5a22abd9..88a0bbba 100644 --- a/src/vector_store/__init__.py +++ b/src/vector_store/__init__.py @@ -180,6 +180,17 @@ class VectorStore(ABC): """ ... + @abstractmethod + async def probe_namespace_dim(self, namespace: str) -> int | None: + """ + Return the declared vector dimension of an existing namespace. + + Returns ``None`` if the namespace does not exist yet (lazy-create + model: not an error). Raises only when the SDK reports the + namespace exists but its schema is unreadable. + """ + ... + def _create_store_by_type(store_type: str) -> VectorStore: """Create a vector store instance by type name.""" diff --git a/src/vector_store/lancedb.py b/src/vector_store/lancedb.py index 13d5124b..8004d8e8 100644 --- a/src/vector_store/lancedb.py +++ b/src/vector_store/lancedb.py @@ -369,3 +369,16 @@ class LanceDBVectorStore(VectorStore): self._db.close() self._db = None logger.debug("LanceDB connection closed") + + async def probe_namespace_dim(self, namespace: str) -> int | None: + """Inspect a LanceDB table's vector column to recover its declared dim.""" + db = await self._get_db() + table_names = await db.table_names() + if namespace not in table_names: + return None + table = await db.open_table(namespace) + schema = await table.schema() + for field in schema: + if field.name == "vector" and hasattr(field.type, "list_size"): + return int(field.type.list_size) + return None diff --git a/src/vector_store/turbopuffer.py b/src/vector_store/turbopuffer.py index 978043aa..0e72c785 100644 --- a/src/vector_store/turbopuffer.py +++ b/src/vector_store/turbopuffer.py @@ -5,6 +5,7 @@ This module provides a Turbopuffer-based implementation of the VectorStore inter """ import logging +import re from collections.abc import Sequence from typing import Any, Literal, cast @@ -307,3 +308,30 @@ class TurbopufferVectorStore(VectorStore): """Close the Turbopuffer client and release resources.""" await self.tpuf.close() logger.debug("Turbopuffer client closed") + + async def probe_namespace_dim(self, namespace: str) -> int | None: + """Inspect a Turbopuffer namespace schema to recover the vector dim. + + Turbopuffer namespaces are lazy-created; ``namespace.exists()`` returns + False before the first write. The schema response maps attribute name + to ``AttributeSchemaConfig``; the vector field's ``type`` string is + a bracket-prefixed dim with a width suffix, e.g. ``"[768]f32"``, + ``"[1536]f16"``, ``"[256]i8"``. + """ + ns = self._get_namespace(namespace) + try: + if not await ns.exists(): + return None + except NotFoundError: + return None + + try: + schema = await ns.schema() + except NotFoundError: + return None + + vector_attr = schema.get("vector") + if vector_attr is None: + return None + match = re.search(r"\[(\d+)\]", str(vector_attr.type)) + return int(match.group(1)) if match else None diff --git a/tests/vector_store/__init__.py b/tests/vector_store/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/vector_store/test_namespace_dim_probes.py b/tests/vector_store/test_namespace_dim_probes.py new file mode 100644 index 00000000..7bc7b5a0 --- /dev/null +++ b/tests/vector_store/test_namespace_dim_probes.py @@ -0,0 +1,72 @@ +"""Per-store namespace dim probe tests. + +LanceDB has an embedded driver we can spin up in a tmp dir, so we exercise +the real probe end-to-end. Turbopuffer needs a network + API key, so it is +covered only by static analysis + the parsing test below. +""" + +from __future__ import annotations + +import re + +import pyarrow as pa +import pytest + +from src.vector_store.lancedb import LanceDBVectorStore + + +@pytest.mark.asyncio +async def test_lancedb_probe_returns_declared_dim( + monkeypatch: pytest.MonkeyPatch, tmp_path: object +) -> None: + """Create a real LanceDB table at dim 768, confirm the probe recovers it.""" + monkeypatch.setattr("src.config.settings.VECTOR_STORE.LANCEDB_PATH", str(tmp_path)) + + store = LanceDBVectorStore() + try: + db = await store._get_db() # pyright: ignore[reportPrivateUsage] + schema = pa.schema( + [ + pa.field("id", pa.string()), + pa.field("vector", pa.list_(pa.float32(), 768)), + ] + ) + await db.create_table("probe_test", schema=schema) + + dim = await store.probe_namespace_dim("probe_test") + assert dim == 768 + finally: + await store.close() + + +@pytest.mark.asyncio +async def test_lancedb_probe_returns_none_for_missing_namespace( + monkeypatch: pytest.MonkeyPatch, tmp_path: object +) -> None: + """Lazy-create model: probing a nonexistent table is not an error.""" + monkeypatch.setattr("src.config.settings.VECTOR_STORE.LANCEDB_PATH", str(tmp_path)) + + store = LanceDBVectorStore() + try: + dim = await store.probe_namespace_dim("does_not_exist") + assert dim is None + finally: + await store.close() + + +def test_turbopuffer_vector_dim_regex_extracts_dim_from_type_string() -> None: + """Turbopuffer's attribute type for a vector column is a bracket-prefixed + dim with a width suffix: ``[768]f32``, ``[1536]f16``, ``[256]i8``. The + probe extracts the integer inside the brackets. Lock the format here so + an SDK change is loud.""" + pattern = re.compile(r"\[(\d+)\]") + cases = { + "[768]f32": "768", + "[1536]f16": "1536", + "[256]i8": "256", + } + for type_str, expected in cases.items(): + match = pattern.search(type_str) + assert match is not None, f"failed to match {type_str!r}" + assert match.group(1) == expected + assert pattern.search("string") is None