Commit Graph

1 Commits

Author SHA1 Message Date
Vineeth Voruganti b84da15d03
Make embeddings configurable (#678)
* feat(embedding): add dimensions_mode for OpenAI dimensions= forwarding

Add EMBEDDING_MODEL_CONFIG__DIMENSIONS_MODE (auto|always|never) controlling
  whether the dimensions= parameter is forwarded on OpenAI embeddings.create
  calls. auto (default) sends it when the operator explicitly set
  EMBEDDING_VECTOR_DIMENSIONS and the configured model is not on the
  known-rejecting allowlist (currently text-embedding-ada-002).

  The provenance check (was VECTOR_DIMENSIONS explicitly set?) lives as
  EmbeddingSettings.resolve_send_dimensions() because it needs access to
  model_fields_set, which the standalone resolver does not have. The
  resolved boolean is passed into _EmbeddingClient at construction time;
  the client never inspects mode or provenance.

  Also pins cloudevents <2.0 — 2.0.0 reorganized the package and dropped
  cloudevents.conversion and cloudevents.http, which src/telemetry/emitter.py
  imports. The original `>=1.12.0` constraint allowed the broken 2.0 resolve.
  With the pin, the imports resolve cleanly and the basedpyright warning
  cascade (37+ warnings about unknown types) disappears.

  Drive-by cleanups (all unnecessary cast/ignore comments flagged by
  basedpyright after the cloudevents downgrade):
  - vector_store/lancedb.py, tests/conftest.py, and
    tests/deriver/test_vector_reconciliation.py — drop dead pyright ignores
  - sdks/python/src/honcho/http/{async_,}client.py — drop unnecessary
    cast(datetime, ...) (parsedate_to_datetime already returns datetime)
  - vector_store/turbopuffer.py — cast(Any, rows) for the upsert_rows
    TypedDict that the SDK exposes but our row builder doesn't satisfy
  - tests/test_datetime_parsing.py — ignore reportArgumentType on the
    test that deliberately passes wrong types to assert raises

* feat(models): honor EMBEDDING_VECTOR_DIMENSIONS in pgvector columns

* feat(startup): atomic swap dim-vs-MIGRATED guard for runtime schema validator

Add src/startup/embedding_validator.py that introspects the actual pgvector
  column dim at boot and refuses to start if it does not match
  EMBEDDING_VECTOR_DIMENSIONS. Runs after the DB pool is up and before the
  embedding client is constructed, in both src/main.py (FastAPI lifespan) and
  src/deriver/__main__.py.

  Implementation details:
  - Schema-qualified pg_attribute join through pg_class/pg_namespace respects
    DB.SCHEMA rather than relying on search_path
  - Bounded retry (3 attempts, 1s backoff) for transient introspection failure,
    then fail-closed with "could not validate embedding schema" — uncertainty
    is not a green light to serve traffic
  - External-store sampler (turbopuffer, lancedb) enumerates workspaces from
    the application DB and probes their lazy-created namespaces; current
    per-namespace probe is a no-op stub since the SDKs do not expose
    uniform dim introspection — full enumeration is left to
    `configure_embeddings --report` in Phase 3

  Atomic guard swap: deletes the old dim-vs-MIGRATED config validator (which
  forbade non-1536 pgvector unless MIGRATED=True) in the same commit as the
  new runtime validator. There is no release window where non-1536 pgvector
  can start unprotected. The 9 dual-write branches that use VECTOR_STORE.MIGRATED
  remain untouched and load-bearing for legacy-tenant backend swaps.

  VECTOR_STORE_DIMENSIONS deprecation: drop the "must match" raise; in
  propagate_namespace, check model_fields_set and emit logger.warning +
  DeprecationWarning (DeprecationWarning alone is filtered by Python's default
  config and would not reach operators). Always overwrite with
  EMBEDDING.VECTOR_DIMENSIONS regardless.

  Test changes:
  - tests/test_models_vector_dim.py: Phase 1's VECTOR_STORE_TYPE=lancedb +
    MIGRATED=true escape hatches removed; the test now passes on plain
    EMBEDDING_VECTOR_DIMENSIONS=768
  - tests/llm/test_model_config.py: the two tests asserting the old guards
    replaced with tests for the new deprecation + acceptance behavior
  - tests/startup/test_embedding_validator.py: 10 new tests — dim assertion
    logic (pass/mismatch/missing/unbounded/non-public-schema), fail-closed
    retry budget, real-test-DB pass, real-DB ALTER-then-validate, deprecation
    warning capture, non-1536 + pgvector + MIGRATED=false at config time

* feat(scripts): add configure_embeddings bootstrap CLI

Adds scripts/configure_embeddings.py alongside the other one-off scripts
  (provision_db, migrate_db, generate_jwt_secret, etc.). Invoked as
  `uv run python scripts/configure_embeddings.py` — same convention as the
  existing scripts in that directory, including the sys.path shim that
  lets src.* imports resolve when run directly.

  Bootstrap step for self-hosted installs at a non-default
  EMBEDDING_VECTOR_DIMENSIONS — runs between `alembic upgrade head` and
  starting the API/deriver.

  pgvector ALTER safety (single transaction):
  - LOCK TABLE {schema}.documents, {schema}.message_embeddings IN ACCESS
    EXCLUSIVE MODE — closes the TOCTOU window between population check
    and ALTER
  - COUNT(*) WHERE embedding IS NOT NULL on both tables; refuse with a
    non-zero exit if either is populated (ALTER ... USING NULL would
    silently wipe those vectors)
  - Snapshot HNSW index DDL from pg_indexes; drop, ALTER, recreate from
    the captured DDL so operator-set HNSW params (m, ef_construction)
    survive the round trip

  External vector stores (turbopuffer, lancedb) are never created or
  modified — namespaces are per-workspace and lazy-created on first write.
  The --report mode enumerates workspaces and collections from the
  application DB, derives the expected namespaces via
  get_vector_namespace(), and prints a per-namespace status table.

  CLI modes (mutually exclusive):
  - (default) interactive: print plan, prompt to confirm
  - --dry-run: print plan and exit 0 without touching the DB
  - --yes: apply without prompt
  - --report: print external-store namespace inventory and exit

  Also updates src/startup/embedding_validator.py error-message paths and
  docs/v3/contributing/configuration.mdx invocations to point at the new
  script location.

  Tests cover plan no-op, plan needs-alter, plan raises on missing column,
  ALTER + HNSW round-trip, refuse-when-populated (monkeypatched count to
  avoid wiring the full workspace/peer/collection/document FK chain just
  to land one vector row), and idempotency.

* docs: add changing-embeddings operations page

Document the supported way to change EMBEDDING_VECTOR_DIMENSIONS or
EMBEDDING_MODEL_CONFIG__MODEL on a Honcho deployment: provision a new
deployment at the desired configuration, replay source data out of
band, cut over at the application layer.

The page explains the asymmetry:
- Dimension is machine-enforced as immutable. The startup validator
  introspects pg_attribute and crashes the API/deriver on mismatch.
- Model is operator-owned. There is no persistent metadata recording
  which model produced each vector, so a same-dim model swap is
  silently undetectable — flagged with a Warning callout.

Also documents the truncation edge case (text-embedding-3-large                                                                                                truncated to 1536 with EMBEDDING_VECTOR_DIMENSIONS left at default)
and the DIMENSIONS_MODE=always mitigation, plus a pointer that
storage-backend swap (VECTOR_STORE_MIGRATED + reconciler) is a                                                                                                 distinct operation unaffected by this work.
Registers the page in docs/docs.json under the Self-Hosting nav group
and cross-links from configuration.mdx.

* fix(embedding): correct turbopuffer regex + tighten DIMENSIONS_MODE docs

- Turbopuffer attribute type for a vector column is `[N]f32` / `[N]f16` /
  `[N]i8`, not `f32_vector(N)` as the earlier probe assumed. The earlier
  regex returned None for the real SDK format, so existing Turbopuffer
  namespaces would have been reported as "missing" instead of validated
  for mismatch. Regex switched to `\[(\d+)\]` which is the
  vendor-stable shape. Test cases rewritten to lock the actual format.

- docs/v3/contributing/configuration.mdx had a contradictory pair of
  bullets: 223 said explicit 1536 makes `auto` forward dimensions=, 224
  said `auto` would skip the parameter because 1536 is the default.
  Operators reading both would (rightly) conclude they need `always`
  even when `auto` would work. Rewrote both bullets so:
  - `auto` is provenance-driven (explicit-set, not non-default-value).
  - `always` is positioned as defense-in-depth for config layers that
    might strip explicit default-valued envs, not the only path for
    same-as-default truncation.

* fix(embedding): address PR #678 review comments

CodeRabbit + Rajat review feedback. All actionable items addressed
except two false-positives (responded on PR).

Bug fixes:
- deriver telemetry leak: validator was called outside try/finally so
  shutdown_telemetry() did not run on validation failure. Moved inside.
- _emit_report printed "no effect with pgvector" unconditionally,
  including from implicit post-apply calls. Added is_report_mode flag;
  only print on explicit --report.
- LanceDB and Turbopuffer probes returned None when the namespace
  existed but its schema was malformed (no vector field / unparseable
  type string), silently bucketing real corruption as "missing"
  (lazy-create) and letting it pass the startup validator. Now raise
  VectorStoreError with actionable diagnostics; None remains valid only
  for "namespace does not exist."
- Startup validator only sampled message namespaces; added a parallel
  Collection-row sample so document namespaces are probed too, with the
  same dim assertion. Mirrors the --report path.

Hygiene:
- StartupValidationError now subclasses HonchoException so existing
  exception handlers recognize it. ValidationException is @final and
  has 422 request-validation semantics that would be misleading here.
- scripts/configure_embeddings.py main() no longer spins up two event
  loops. engine.dispose() moved into a try/finally inside _async_main
  so cleanup runs in the same loop as the pipeline.
- Replaced hand-rolled retry loop with tenacity.AsyncRetrying; same
  fail-closed semantics, less code, before_sleep_log for visibility.
- Added _validate_identifier() defense-in-depth: DB.SCHEMA and HNSW
  index names are regex-checked against [A-Za-z_][A-Za-z0-9_]* before
  SQL interpolation. Operator config + DB catalog are not user input
  under the current threat model, but the constraint is cheap to gate.

Test + docs:
- test_app_settings_accepts_non_1536_with_any_vector_store_configuration
  now actually exercises turbopuffer (was missing); supplies a dummy
  TURBOPUFFER_API_KEY to satisfy the model_validator.
- changing-embeddings.mdx: hyphenated "out-of-band" per reviewer style.

* fix: modify conftest to fix ci

* fix: ci tests for typescript server
2026-05-14 15:03:35 -04:00