Commit Graph

13 Commits

Author SHA1 Message Date
Ken Weiner 5823f0fae9
fix: only classify genuine oversize input as a token-limit error (#791)
Callers wrapped every ValueError from the embedding client in a
"exceeds maximum token limit" message, so provider and configuration
failures (dimension mismatch, empty response, upstream error) surfaced
to users as though their input were too long.

Add EmbeddingTokenLimitError, raised only by the pre-flight token checks
in embed() and simple_batch_embed(), and narrow the remaps in search.py,
agent_tools.py, document.py and representation.py to catch it. It
subclasses ValueError so existing broad handlers keep working.

Both simple_batch_embed() remap sites pass on_oversize="truncate" and so
could never raise a token-limit error at all; their handlers only ever
mislabelled provider failures.

Fixes #568

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-08-25 09:45:01 -04:00
Aakash Kattelu ddbb90e36f
fix(embedding): truncate in batch embed and return results breakdown (#1019)
* fix(deriver): truncate oversize observations so one cannot drop the batch

simple_batch_embed raised ValueError when any input exceeded the per-input
token cap, which failed the entire deriver save when a single observation
was over-length. Add on_oversize="truncate": oversize inputs are embedded
from a token-capped prefix (re-encoded until it fits, with a warning),
preserving one vector per input. Default stays "raise" so existing callers
are unchanged. RepresentationManager opts into truncate.

Also add a live embedding test that fails on main (raise / missing kwarg)
and passes once a mixed short+oversize batch survives.

Refs #569

* fix(deriver): surface failure when all observer saves fail

When every observer's save_representation failed (e.g. embedding retries
exhausted under a sustained 429), the deriver logged the error and returned
normally, so the queue marked the work unit processed with zero documents
saved. Collect per-observer errors and, after telemetry is emitted, raise
RepresentationSaveError when no observer succeeded. Partial failures stay
processed (saved observers must not be discarded) and are recorded via an
additive failed_observer_count on RepresentationCompletedEvent.

Refs #728

* fix(embedding): guarantee truncation progress and truncate on re-embed

The retry slice in _truncate_to_token_limit always recomputed the same
keep count, so a slice whose re-encode grew past the cap could oscillate.
Decrement keep after each unsuccessful retry.

Document re-embed in the reconciler used the default on_oversize="raise",
so one oversize document failed every other document in the batch.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* chore: drop ticket ids and shrink comments to one sentence

Comments and docstrings describe current behavior, not the PR that
introduced them. Ticket numbers stay in the commit/PR.

* chore: annotate RepresentationSaveError and assert truncate on re-embed

* fix(embedding): truncate on conclusion create paths and document BPE loop

Storage callers in create_observations (API + agent tools) now pass
on_oversize="truncate" so a single oversize item cannot drop the batch.
Docstring on _truncate_to_token_limit notes why decode/re-encode is load-bearing.

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-08-20 11:42:38 -04:00
Joe-Kneeland 2163ab1aa3
fix(llm): forward provider_params.timeout to the OpenAI-compatible embedding client (#1024)
* fix(llm): forward provider_params.timeout to the OpenAI-compatible embedding client

#832 and #903 added a configurable request timeout for the LLM registry
and the Gemini embedding client respectively, but the OpenAI-compatible
embedding client (src/embedding_client.py) was never wired up. It
constructed AsyncOpenAI with no timeout at all, so a stalled socket
against a slow or contended OpenAI-compatible backend (e.g. a
self-hosted embedding model under load) wedges the deriver worker's
event loop indefinitely — the exact failure #785/#903 describe, just
via a code path #903 didn't cover.

EmbeddingModelConfig now carries provider_params through from
resolve_embedding_model_config, mirroring how resolve_model_config
already does it for ModelConfig, and the OpenAI branch of
_EmbeddingClient.__init__ extracts `timeout` via the existing
request_timeout_from_extra_params helper. Unset stays unset — no
existing behavior changes.

Reproduced and verified against a real self-hosted deployment (local
Ollama backend under load): before this fix, a single stuck embedding
call blocked all deriver queue processing for 20+ minutes with no
error logged, twice in one session.

* fix(embedding): use first-class timeout on embedding model config

provider_params is the LLM per-request escape hatch; embedding timeouts are
client-construction knobs and belong next to max_batch_size. Wire the field
for OpenAI and Gemini, omit the OpenAI kwarg when unset so the SDK default
stays, and keep Gemini's 10-minute floor when unset.

* test(embedding): live coverage for first-class embedding timeout

Exercise EmbeddingModelConfig.timeout on one representative OpenAI and
Gemini model: configured timeout lands on the SDK client, and a near-zero
timeout aborts before the provider answers.

---------

Co-authored-by: Aakash Kattelu <aakash@plasticlabs.ai>
2026-08-18 10:51:53 -04:00
Aakash Kattelu 252269e9b6
perf: lazy-load provider SDKs to cut idle memory per process (#1011)
Import anthropic/openai/google-genai only when a provider is first used
instead of at module import. CLIENTS is now populated lazily via
default_client(), which preserves the patch.dict test seam. The
embedding client defers its SDK imports the same way and dispatches on
transport instead of isinstance.

Cuts idle RSS by ~60MiB per process with all three providers configured
but unused at startup; a process that only ever calls one provider also
never pays for the other two.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-08-12 21:39:12 -04:00
Vansh Sharma 7b8c2917f9
fix(embedding): request float encoding_format on openai embedding calls (#938)
* test(embedding): add reproducer for missing encoding_format on openai paths

The openai SDK defaults encoding_format to base64 when it is not passed. OpenAI-compatible providers that don't support base64 embeddings (e.g. OpenRouter with nvidia/nemotron-3-embed-1b:free) return HTTP 200 with empty data, and every embedding call fails with 'No embedding data received'.

* fix(embedding): request float encoding_format on openai embedding calls

The openai SDK defaults encoding_format to base64 when the caller does not pass one. OpenAI-compatible providers that don't support base64 embeddings (e.g. OpenRouter hosting nvidia/nemotron-3-embed-1b:free) answer HTTP 200 with empty embedding data, and every embedding call fails with 'No embedding data received', breaking conclusions, semantic search, and the deriver. Pass encoding_format='float' explicitly on both the single-query and batch call paths.

* test(embedding): cover openai-compatible providers in the live embedding matrix

The existing openai family runs against real OpenAI, which serves base64
embeddings happily, so the matrix passes with or without the #932 fix. Adds an
`openai_compatible_embedding` family (openai transport, third-party base_url)
so the matrix can reach a provider that rejects base64. Empty default_models
keeps it skipped unless LIVE_EMBEDDING_OPENAI_COMPATIBLE_MODELS is set.

Also adds test_live_openai_float_encoding_matches_base64, which pins the other
direction: switching the wire format must not move vectors on real OpenAI.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(embedding): keep an explicit embedding-count check on the openai paths

Passing `encoding_format` disables the openai SDK's own empty-data guard, so a
provider answering 200 with missing embeddings surfaced as `IndexError: list
index out of range` on the single path and `zip() argument 2 is shorter than
argument 1` on the batch path. The latter is also #745's signature, which would
have left it with two unrelated causes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs(live-llm): correct the openai-compatible embedding matrix env docs

The documented default dimensions said 2048 after the family moved to 3072, and
LIVE_EMBEDDING_OPENAI_COMPATIBLE_SEND_DIMENSIONS was missing entirely. Also
points the example and the coverage note at a model that is actually reachable,
and records that OpenRouter load-balances, so the base64 failure is per-attempt.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* feat(embedding): resolve openai encoding_format by mode instead of pinning float

Requesting float unconditionally costs ~3.6x the response bytes of base64 and up
to +83% latency on a 500-item batch, which the default deployment on real OpenAI
pays for nothing: only third-party OpenAI-compatible providers reject base64.

Adds EMBEDDING_MODEL_CONFIG__ENCODING_FORMAT_MODE, mirroring dimensions_mode.
`auto` keeps base64 when no base_url override is set or it points at
api.openai.com, and picks float elsewhere. The format is still always sent
explicitly, since the SDK otherwise injects base64 on its own.

Also corrects the _validate_embedding_count docstring, which said "fewer" where
the guard is an inequality.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(embedding): request base64 embeddings by omission, not by name

The openai SDK decodes a base64 response only when it injected the default
itself; naming any format makes it skip the decode and hand back the raw string,
which then fails the dimension check with "Expected 1536, got 8192". base64 mode
therefore has to omit the kwarg rather than pass it.

The unit fake returned float lists whatever was asked for, so it could not catch
this. It now mirrors the SDK and returns a base64 string for a named base64
request, which fails against the previous commit.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Aakash Kattelu <aakash@plasticlabs.ai>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-08-12 13:24:38 -04:00
Niyaz Almufti a92fb1e078
Fix Gemini batch embedding for gemini-embedding-2* models (#745)
* Fix Gemini batch embedding for gemini-embedding-2* models

The google-genai SDK treats embed_content(contents=[list_of_strings]) as a
single multi-part document for gemini-embedding-2* models, silently returning
exactly 1 embedding regardless of input count. This caused a zip(...,
strict=True) ValueError in _process_batch.

Wrap each text in genai_types.Content(parts=[genai_types.Part(text=...)])
so the SDK treats each string as a separate content item. This matches the
workaround used by pydantic-ai (#4873) and graphiti (#1474).

Upstream SDK issue: googleapis/python-genai#2523
Fixes plastic-labs/honcho#744

* test(embedding): live embedding coverage for every Gemini and OpenAI model

Adds tests/live_llm/test_live_embeddings.py plus an env-driven embedding
matrix alongside the existing LLM one. Covers single embed, batched embed,
batch-vs-single alignment, chunk-to-id mapping, and the batch-split path.

Only a live call catches the gemini-embedding-2* collapse: the SDK folds a
list of bare strings into one document and returns a single embedding.
Reverting the Content wrapping fails all four batch tests for
gemini-embedding-2-preview and gemini-embedding-2 with the reported
`zip() argument 2 is shorter than argument 1`, while gemini-embedding-001
and text-embedding-3-small stay green.

Also makes the concatenation in the conclusions semantic-search validation
message explicit, so the repo-wide basedpyright pre-push hook passes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(embedding): drop the preview twin from the default embedding matrix

gemini-embedding-2 is the GA release of gemini-embedding-2-preview and
behaves identically, so running both by default doubles the Gemini cost for
no extra coverage. The preview stays reachable through
LIVE_EMBEDDING_GEMINI_MODELS.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Aakash Kattelu <aakash@plasticlabs.ai>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-08-11 17:50:40 -04:00
JUNZE 00d6d36728
fix: make embedding batch size configurable (#983)
* fix: resolve tiktoken encoding without constructing the embedding client

EmbeddingClient.encoding forced full client construction, which raises
'OpenAI API key is required' even though tiktoken needs no credentials.
The document dedup tie-break (src/crud/document.py) only needs .encoding
for token counting, so any test hitting that path fails in environments
without embedding keys — notably CI for pull requests from forks, where
repo secrets are unavailable (e.g. #908's test-python job failing on
tests/crud/test_document.py::test_duplicate_rejection_reinforces_existing).

Resolve the encoding from the configured model directly, falling back to
cl100k_base, and only reuse the underlying client's encoding when it has
already been constructed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix: make embedding batch size configurable

Add optional max_batch_size to the embedding model config
(EMBEDDING_MODEL_CONFIG__MAX_BATCH_SIZE) to cap texts per request for
OpenAI-compatible providers with smaller limits than OpenAI's, such as
DashScope text-embedding-v4 (10) and Alibaba Bailian
qwen3.7-text-embedding (20). When unset, native provider defaults are
preserved (OpenAI 2048, Gemini 100).

Fixes #687.

* test(embedding): cover Gemini batching and config fallbacks per review

- Gemini transport now tested for configured batch splitting and the 100
  default fallback
- OpenAI unset default (2048, single request) explicitly covered
- env-parsing test now asserts the value survives resolve_embedding_model_config
- docs: 100 is the client's conservative Gemini default, not a native limit

* test(embedding): assert provider batch-size defaults

---------

Co-authored-by: adavyas <adavyasharma@gmail.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-08-05 15:43:32 -04:00
Aakash Kattelu e6d4d78ba1
fix(ci): clear basedpyright warnings from #903 tests; run static analysis on PRs (#975) 2026-08-04 17:03:10 -04:00
PK 5c32bd10ec
fix(llm): set HTTP timeout on Gemini clients (#903)
* fix(llm): set HTTP timeout on Gemini clients (#785)

* fix(embedding): set HTTP timeout on Gemini embedding client (#785)

Same wedge-class failure as the LLM client: a stalled Gemini embedding
socket hangs the in-process reconciler, which shares the deriver worker's
uvloop event loop. Apply the same 10-minute timeout here, in lockstep
with src/llm/registry.py's _build_gemini_http_options.

* style(test): drop extra blank line in test_registry imports
2026-08-04 16:29:32 -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
qxxaa 1c3e3f8816
fix: embed() sends string input instead of array, breaking OpenAI-compatible providers (#586)
* fix: wrap single embed() input in array for OpenAI-compatible provider compatibility

* Fix input format in embedding test assertion
2026-04-20 16:35:13 -04:00
Vineeth Voruganti b65d03d297
Refactor clients.py to add modern features and more flexible configuration (#459)
* fix: Add JSON repair for truncated LLM responses across all providers and Gemini thinking budget support

LengthFinishReasonError from OpenAI-compatible providers (custom, openai, groq) was crashing the deriver
with 14k+ occurrences in production. The vLLM path already had repair logic but it was gated on
provider=="vllm", unreachable when routing through litellm as a custom provider.

- Extract shared _repair_response_model_json() helper for all providers
- Catch LengthFinishReasonError in OpenAI/custom parse() path and repair truncated JSON
- Add repair fallback to Anthropic and Gemini response_model paths
- Add repair fallback to Groq response_model path
- Pass thinking_budget_tokens to Gemini 2.5 models via thinking_config
- Add 14 tests covering repair paths for all providers and Gemini thinking budget

Fixes HONCHO-YC

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

* feat: live llm integration tests

* feat: Consistent Model Config Protocol

* fix: migrate the remaining app callers off the legacy llm_settings path

* fix: Docs and regression tests

* fix: refactor llm runtime path to model-config-only API

* fix: refactor config to nested model-config source of truth

* fix: refactor llm streaming and tool dispatch through backends

* fix: cut over llm config to nested model_config only

* fix: collapse vllm and custom into openai_compatible transport

* feat: refactor llm config to explicit transports and bare model ids

* feat: (embed) Add configurability for embedding model

* fix: tests for embedding provider

* fix: Address Review Comments

* fix: (llm) remove Groq backend and per-vendor base URLs

* chore: move llm tests

* fix: (llm) address review findings — config regressions, backend bugs, dead code

* fix: address backend end silly errors

* chore: (docs) update configuration and self-hosting guides

* chore: fix tests

* fix: address code rabbit comments

* fix: add validation to the dream settings

* fix: further address code rabbit comments

* fix: Address Code Rabbit Comments

* fix: Another round of code rabbit

* fix: Address Code Rabbit Nits

* fix: tests

* refactor: rename thinking validator to reflect transport scope

_validate_anthropic_thinking_minimum only enforces the >=1024 rule for
Anthropic and no-ops for other transports, so the name was misleading
now that it's shared across ConfiguredModelSettings, FallbackModelSettings,
and ModelConfig. Renamed to _validate_thinking_constraints with a docstring
clarifying per-transport behavior. No logic change.

* fix(config): drop transport-specific thinking params when env override changes transport

_fill_defaults_for_nested_field previously preserved the default MODEL_CONFIG's
thinking_budget_tokens/thinking_effort across a transport override. This leaked
Gemini-family defaults (e.g. thinking_budget_tokens=1024) into OpenAI-transport
overrides, and the OpenAI backend then correctly rejected the unsupported param
at call time (OpenAI uses reasoning.effort, not a token budget).

The helper now strips thinking_budget_tokens and thinking_effort from the
default dict when the env override supplies a transport different from the
default's. Explicit thinking params in the override are preserved.

* fix(config): apply thinking-param strip to dialectic level merge too

DialecticSettings._merge_level_defaults does its own inline MODEL_CONFIG
merge (parallel to _fill_defaults_for_nested_field), so the previous fix
missed dialectic-level overrides. E.g. flipping
DIALECTIC_LEVELS__minimal__MODEL_CONFIG__TRANSPORT from gemini (default)
to openai still leaked the default thinking_budget_tokens=0 into the
openai config, which the OpenAI backend then rejected at call time.

The level-merge path now applies the same 'strip transport-specific
thinking params when transport changes' rule as the generic helper.
Added a regression test exercising the merge validator directly.

* refactor(llm): wire ModelConfig knobs through, prune clients.py migration leftovers

Three connected fixes to finish carving the LLM stack out of src/utils/clients.py
and into src/llm/:

1. Propagate ModelConfig tuning knobs into backend calls.
   honcho_llm_call_inner built extra_params from only {json_mode, verbosity},
   silently dropping top_p, top_k, frequency_penalty, presence_penalty, seed,
   and operator-supplied provider_params from any ModelConfig. Thread the
   selected config through ProviderSelection and merge
   build_config_extra_params(selected_config) into extra_params; per-call
   kwargs still win over provider_params defaults. Makes
   _build_config_extra_params public as build_config_extra_params so
   clients.py and request_builder.py share one translation. Adds
   TestModelConfigExtraParamsPropagation covering OpenAI/Anthropic knob
   propagation, provider_params passthrough, and per-call override
   precedence.

2. Drop dead extract_openai_* duplicates in clients.py.
   extract_openai_reasoning_content, extract_openai_reasoning_details, and
   extract_openai_cache_tokens had no callers outside their own definitions
   — the live implementations live in src/llm/backends/openai.py. -103
   lines from clients.py.

3. Unify on ModelTransport, delete SupportedProviders.
   The "google" vs "gemini" split forced a _provider_for_model_config
   translation shim in two places. Replace all SupportedProviders usages
   with ModelTransport, rename CLIENTS["google"] → CLIENTS["gemini"],
   update provider branches + LLMError labels + reasoning-trace entries
   accordingly. Trace JSONL now writes "provider": "gemini" instead of
   "google" — consistent with the broader env-var rename cutover.

Also tidies up pre-existing basedpyright findings in tests/llm/test_model_config.py
(pydantic before-validator dict inputs + descriptor-proxy call).

ruff: clean. basedpyright: 0 errors, 0 warnings. Tests: 153/153 pass across
tests/utils/test_clients.py, tests/utils/test_length_finish_reason.py,
tests/llm/, tests/dialectic/, tests/deriver/.

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

* refactor(llm): finish the src/utils/clients.py → src/llm/ migration

honcho_llm_call_inner now delegates to request_builder.execute_completion
and execute_stream instead of re-implementing backend call scaffolding
inline. The new _effective_config_for_call helper carries per-call kwargs
(temperature, stop_seqs, thinking_budget_tokens, reasoning_effort) onto
the selected ModelConfig — or synthesizes a minimal config for the
test-only callers that pass provider+model directly. max_output_tokens
is zeroed on the effective config to preserve the current
"per-call max_tokens wins" semantic; honoring ModelConfig.max_output_tokens
is a separable correctness concern.

Side effect of routing through the new path: ConfiguredModelSettings'
thinking_budget_tokens validator now fires on synthesized configs.
test_anthropic_thinking_budget was asserting that a sub-1024 budget
propagated to Anthropic — bumped to 1024 to match what Anthropic actually
accepts.

Unified client construction. Promoted the cached client factories in
src/llm/__init__.py (get_anthropic_client, get_openai_client,
get_gemini_client, get_{anthropic,openai,gemini}_override_client) to
public API and added them to __all__. Promoted
credentials._default_transport_api_key → default_transport_api_key.
Deleted the duplicate _build_client and _default_credentials_for_provider
from clients.py; _client_for_model_config now falls through to the
public factories. CLIENTS dict and _get_backend_for_provider stay as the
mockable seam for the ~50 patch.dict(CLIENTS, {...}) test call sites.

Wired operator-configurable Gemini cached-content reuse end-to-end.
PromptCachePolicy moved from src/llm/caching.py into src/config.py so
ModelConfig can reference it as a field without a circular import;
caching.py re-exports the name for existing imports. Added
cache_policy: PromptCachePolicy | None on ConfiguredModelSettings,
FallbackModelSettings, ResolvedFallbackConfig, and ModelConfig.
resolve_model_config, _resolve_fallback_config, and
_select_model_config_for_attempt copy the field through.
honcho_llm_call_inner passes effective_config.cache_policy into
execute_completion / execute_stream, so operators opt in via
e.g. DERIVER_MODEL_CONFIG__CACHE_POLICY__MODE=gemini_cached_content
and the selection actually fires instead of sitting on a dead path.

New regression test test_cache_policy_reaches_gemini_backend asserts the
PromptCachePolicy object reaches the Gemini backend's extra_params.

ruff + basedpyright: clean. Tests: 154/154 pass across
tests/utils/test_clients.py, tests/utils/test_length_finish_reason.py,
tests/llm/, tests/dialectic/, tests/deriver/.

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

* refactor(llm): move all LLM orchestration into src/llm/ and delete clients.py

The 1624-line src/utils/clients.py has been carved up into focused modules
under src/llm/ and deleted. There is now one golden path for LLM
orchestration and no dual entrypoint.

New module layout:

  src/llm/
    __init__.py       thin stable re-export surface
    api.py            public honcho_llm_call with retry + fallback + tool
                      loop delegation
    executor.py       honcho_llm_call_inner (single-call executor); bridges
                      to request_builder.execute_completion / execute_stream
    tool_loop.py      execute_tool_loop + stream_final_response, plus
                      assistant-tool-message and tool-result formatting
    runtime.py        AttemptPlan dataclass (replaces the loose
                      ProviderSelection NamedTuple), effective_config_for_call,
                      plan_attempt, per-retry temperature bump, attempt
                      ContextVar
    registry.py       single owner of CLIENTS dict + cached default and
                      override SDK-client factories + backend/history-adapter
                      selection + high-level get_backend(config)
    conversation.py   count_message_tokens, tool-aware message grouping,
                      truncate_messages_to_fit
    types.py          HonchoLLMCallResponse, HonchoLLMCallStreamChunk,
                      StreamingResponseWithMetadata, IterationData,
                      IterationCallback, ReasoningEffortType, VerbosityType,
                      ProviderClient
    request_builder.py low-level request assembly (ModelConfig → backend
                      complete/stream); no longer owns credential resolution
    credentials.py    default_transport_api_key, resolve_credentials
    caching.py        gemini_cache_store; re-exports PromptCachePolicy
                      from src.config
    backend.py        Protocol + normalized result types
    history_adapters.py provider-specific assistant/tool message shapes
    structured_output.py
    backends/         AnthropicBackend, OpenAIBackend, GeminiBackend

handle_streaming_response had no production callers; it is deleted. The
three tests that used it now drive honcho_llm_call_inner(stream=True,
client_override=...) directly, which exercises the same code path the
public API uses.

Dead credential passthrough removed. The ProviderBackend Protocol and
all three concrete backends no longer accept api_key / api_base — those
are baked into the underlying SDK client at registry construction time
and were being del'd everywhere they appeared. request_builder also
stops resolving and forwarding them.

Client construction is unified. The cached default-client factories
(get_anthropic_client, get_openai_client, get_gemini_client) and override
factories (get_*_override_client) are promoted to public API; the
module-level CLIENTS dict populates from them and remains the
patch.dict(CLIENTS, {...}) mocking seam tests rely on. Old duplicate
helpers (_build_client, _default_credentials_for_provider) are gone.
default_transport_api_key is promoted to public.

Application imports now come from src.llm (dreamer, dialectic, deriver,
summarizer, telemetry-adjacent tests). No code imports from
src.utils.clients anywhere in the repo.

ruff: clean. basedpyright: 0 errors, 0 warnings. Tests: 1013/1013 pass
across the entire non-infra test suite (excluding tests/unified,
tests/bench, tests/live_llm, tests/alembic).

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

* fix(llm): sanitize tool schemas for Gemini's function_declarations validator

Gemini's native-transport function-declarations validator accepts a narrow
subset of JSON-Schema / OpenAPI: type, format, description, nullable, enum,
properties, required, items, minItems, maxItems, minimum, maximum, title.
Anything else — additionalProperties, allOf, if/then/else, $ref, anyOf,
oneOf, $defs, patternProperties — triggers an INVALID_ARGUMENT 400 at call
time.

Our agent tool schemas in src/utils/agent_tools.py use several of those
(additionalProperties: false, allOf + if/then conditionals) because they
were authored for OpenAI strict-mode + Anthropic, which need the richer
vocabulary. GeminiBackend._convert_tools was passing them straight through.

Add _sanitize_schema(): walks the parameters tree and drops unsupported
keywords while preserving semantics for the keywords that hold user data
(properties maps field-name → sub-schema; required / enum are lists of
literals; items is a single sub-schema). Other backends are untouched and
continue to receive the full strict schemas.

Regression tests:
- test_gemini_sanitize_schema_strips_unsupported_keywords: confirms
  additionalProperties, allOf + if/then, and $defs are stripped at nested
  levels while legitimate fields survive.
- test_gemini_convert_tools_sanitizes_parameters_schema: end-to-end
  _convert_tools output has no forbidden keys.

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

* fix: fix tool calling syntax for gemini

* refactor(llm): normalize defaults, widen OpenAI reasoning-model routing

* chore: fix test

* fix(llm): address post-migration review feedback

* fix(llm): gemini robustness + dreamer specialist ergonomics

* chore: addres review comments

* chore: (docs) unrelease changelog addition

* chore: (docs) merge commit changes

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Erosika <eri@plasticlabs.ai>
2026-04-20 02:46:37 -04:00