Commit Graph

7 Commits

Author SHA1 Message Date
Vineeth Voruganti e2ff106f28
Filter noisy sentry traces/profiles (#834)
* perf(reconciler): only trace Sentry transactions when work is found

The reconciler enqueues sync_vectors every ~5 min per deriver instance.
process_item wrapped every dequeued reconciler task in a single
process_reconciler_task transaction, so idle cycles (the common case,
where the cycle finds no rows and exits immediately) still created and
sampled a transaction + profile, draining Sentry tracing/profiling quota.

Remove the top-level transaction and push tracing into the sync batch
helpers, starting a per-batch transaction only after rows are confirmed.
Idle cycles now emit zero transactions; busy sweeps emit one smaller
transaction per batch operation.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* perf(telemetry): drop infra/scrape transactions via a Sentry traces sampler

Sentry was sampling every transaction at a flat traces_sample_rate with no
sampler. The Prometheus /metrics scrape endpoint alone accounted for ~92% of
all traced transactions (and their profiles), with /openapi.json and the
deriver metrics server adding more pure noise.

Add a traces_sampler that returns 0.0 for infra/scrape endpoints (/metrics,
/health, /openapi.json, /docs, /redoc, and metrics/openapi transaction names)
and the configured rate for real traffic. Sampling here (vs
before_send_transaction) means dropped transactions are never recorded or
profiled and the decision propagates to child spans. Shared init covers both
the API server and the deriver worker.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-22 21:14:55 -04:00
Rajat Ahuja 6aa6033a16
feat: defer embedding messages (#704)
* feat: defer embedding messages

* fix: rm gauges

* feat: embed messages immediately on create with reconciler fallback (#766)

Adds embed_messages_now background task so newly created messages are
searchable within seconds instead of waiting up to the reconciler
interval. Three-phase claim/lease → embed → persist never holds a DB
session across the embedding call; the reconciler remains the fallback
for failures and stragglers.

* fix: harden immediate-embed fast path and cover its error branches

Wrap embed_messages_now in a top-level try/except so a failure in the
claim or persist phase degrades to "reconciler will retry" instead of
escaping into the background-task runner; the rows stay pending+leased
and the reconciler heals them.

Add tests for the previously-uncovered branches: external-store-unavailable
persist path, the file-upload endpoint's embed scheduling, and direct unit
tests for the shared compute_chunk_positions / build_message_vector_record
helpers.

Document the semantic-search eventual-consistency window in search.mdx
(keyword matches are immediate; vector matches lag creation by seconds).

* fix: don't hold DB session across vector-store upserts

* fix: align semantic-search function to filter null rows

---------

Co-authored-by: Vineeth Voruganti <13438633+VVoruganti@users.noreply.github.com>
2026-06-11 10:31:04 -04:00
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
Rajat Ahuja 03a2374ea1
fix: give vector sync a substantial retry budget (#604) 2026-04-28 16:01:33 -04:00
Rajat Ahuja 7fae16b351
handle turbopuffer server errors (#561)
* fix: catch InternalServerError from turbopuffer

* fix: remove unused VectorUpsertResult

* fix: downgrade vector store sync errors to warnings

* fix: remove upsert_with_retry

* fix: (vector) add silent path and explicit path for vector db server errors

---------

Co-authored-by: Vineeth Voruganti <13438633+VVoruganti@users.noreply.github.com>
2026-04-20 16:56:51 -04:00
Vineeth Voruganti beb282bfbc
fix: Various Codex Audits (#386)
* fix: Various Codex Audits

* fix: Address Comments
2026-02-13 12:00:15 -05:00
Rajat Ahuja 833a89e70a
Turbopuffer and LanceDB Integration (#287)
* feat: init turbopuffer and lanceDB

* fix: remove destructive embedding migration

* fix: bug fixes

* fix: LanceDB

* fix: turbopuffer

* fix: search and add create_observations

* fix: use Async clients

* fix: search; protect agaainst failed vector create/delete

* fix: coderabbit comments

* fix: set up compose vector store and reconciliation loop

* feat: sync docs without embeddings

* fix: reduce batch size; comments; types; add indexes for reconciliation

* fix: add message embedding resilience

* fix: clean-up and migration test

* fix: cleanup 2

* fix: centralize retry logic; bump reconciliation batch; use tracked db; fix soft-delete race condition

* fix: skip double query when pgvector is primary

* fix: down migration

* fix: remove hard-delete from critical path and make PgVectorStore deletions a no-op

* fix: use soft-delete pattern for duplicate detection

* fix: steps toward deprecating MessageEmbedding table

* fix: remove composite and pgvector store -> make more specific

* fix: migration order

* fix: shorten reconciliation cycle + fix 'IN' equality check

* fix: coderabbit comments

* fix: add test for migration 7c0d9a4e3b1f

* feat: refactor to use ReconcilerScheduler

* fix: CR / opus comments

* fix: work unit key and reserve system workspace

* fix: make workspace_name nullable

* fix: clean up sync vectors

* fix: delete syntax

* fix: hash namespace

* External Vector Store Nits (#332)

* fix: Migration naming and long held connection

* chore: Comment for potential debt

* chore: update typescript core package

---------

Co-authored-by: Vineeth Voruganti <13438633+VVoruganti@users.noreply.github.com>
2026-01-16 17:04:01 -05:00