Commit Graph

2 Commits

Author SHA1 Message Date
Vineeth Voruganti e4da919e0b 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.
2026-05-14 12:58:17 -04:00
Vineeth Voruganti 4e80dc05c7 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.
2026-05-13 10:35:51 -04:00