Make the embed endpoint optional so lexical-only search works - #34
Merged
Conversation
TheGreatAxios
added a commit
that referenced
this pull request
Aug 19, 2026
Adversarial review: "string >= 1" on ResolvedCallerSchema is a LENGTH constraint, not a content one -- " " has length 1 and passes it, so a whitespace-only tenantId/principalId was still seated as a "valid" scope. Same bug class PR #34 fixed in optionalEnv (v.length > 0 accepted " "). Extends the malformed-output regression test (deps.test.ts and the end-to-end test in routes.test.ts) to cover a whitespace-only id alongside the empty-string case. Red against the current resolveCaller, which still seats it; the next commit rejects it.
loadMemoryConfig should construct without EMBED_BASE_URL/EMBED_MODEL, reject exactly one of the pair being set, and toEmbedClientConfig should return undefined when embed is unconfigured.
loadMemoryConfig no longer requires EMBED_BASE_URL/EMBED_MODEL. EngineConfig.embed is now optional, and every construction site that consumed it degrades cleanly instead of gating on it: - hybridSearch skips the dense channel entirely when no embed client config resolves (never dispatches a doomed HTTP call), and reports degraded: ["dense_unavailable", "lexical_only"] — dense_unavailable is the existing "dense contributed nothing" signal, lexical_only is new and marks the structural "no embed endpoint at all" case distinctly from a configured endpoint that's merely down. - captureDocument still writes documents when embed is unconfigured; chunks are stored, vectors are not, and the embed-model registry (ensureEmbedModel/activateEmbedModel) is never reached. - The replay/backfill pipeline (runTransform, promoteGeneration) still requires an embed endpoint — re-deriving a corpus is inherently a re-embedding operation — and now fails loudly and early with a clear message instead of throwing deep inside buildEmbedClientConfig. lexical_only is added to DegradeFlag / DEGRADE_FLAG_SET, which the degrade-metrics module's compile-time `satisfies` check already forces for any new flag.
…only EMBED_BASE_URL hybridSearch had no coverage at all for the embed-unconfigured branch, the PR's actual deliverable. Adds real tests against a fake Db/RawSql: lexical results still come back, both dense_unavailable and lexical_only are reported, and no embed HTTP call or embed-model registry access happens (the RawSql fake throws if touched at all). A static import of hybridSearch here collided with mock.module calls in memory.test.ts (Bun's module registry is process-global) once this file started using the real function; a cache-busted dynamic import, the same trick memory.test.ts already uses for memory.ts, sidesteps it. Also covers EMBED_BASE_URL=" " (whitespace-only) being treated as unset rather than a blank baseUrl that fails later with a confusing network error.
optionalEnv checked v.length > 0 but not v.trim(), so
EMBED_BASE_URL=" " slipped past the both-set-or-both-unset guard as
a real value instead of being treated as unset - it would only fail
later, at the first dense HTTP call, with a confusing network error.
Now consistent with requireEnv, which already trims.
loadMemoryConfig also built its return value with
`...(embed ? { embed } : {})`, which AGENTS.md rules out ("explicit
literals, never assembled by spreading"). Replaced with two explicit
object literals, one per branch, matching the `rerank: {...}` literal
in the same function.
dense_unavailable now also fires on every search for a deliberately lexical-only engine, not only on a transient failure - a host with an existing alert keyed on it alone should also check lexical_only.
…m embed guards - embedInsertedChunksWithConfig (capture.ts) now returns a reason array, not a boolean; covers the empty-chunks and unconfigured-embed cases. - createMemory's returned Memory.capabilities.embeddingsConfigured reflects EngineConfig.embed, for both engine and custom-store construction. - runTransform and promoteGeneration's embed-absent guards had zero coverage; adds fake-Db-backed tests for both, asserting on the captured .set() payload rather than a read-back through the same fake. - routes.test.ts's stub Memory needed a capabilities field to satisfy the now-required Memory.capabilities type.
…unify embed vocabulary - add's degraded is now a reason array (CaptureDegradedReason[]: "embed_unavailable" / "lexical_only"), matching search's DegradeFlag[] shape instead of a bare boolean — a host can now write one "is this response degraded" check across both verbs. Threaded through CaptureResult -> DocumentStoreAddResult -> MemoryAddResult -> the /add HTTP route. CaptureDegradedReason lives in core/embed-worker.ts (not services/capture.ts) so ports/types.ts can reference it the same way it already references DegradeFlag for DocumentStoreSearchResult. - Memory.capabilities.embeddingsConfigured (backed by an optional DocumentStore.capabilities) lets a host learn recall is lexical-only at construction time, without issuing a search first. The engine store always reports it; a custom store that omits it defaults to true (the pre-CL-6287 assumption, since this SDK can't otherwise introspect a vendor store). - Settled on "embed endpoint" over "embedding account" throughout (config.ts, engine-client-config.ts, mount-config.ts, IMPLEMENTATION.md) — it's what the config actually is.
The caller-resolver suite (CL-6286) builds a fake Memory that predates the capabilities surface CL-6287 adds, so the two only stop typechecking once both land on main.
TheGreatAxios
force-pushed
the
cl-6287-optional-embed
branch
from
August 19, 2026 00:41
d070f90 to
6163976
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The engine already degrades to lexical-only cleanly at runtime —
hybridSearchfalls back to Postgres full-text search when dense retrieval fails, andembedChunksreports aclientErrorinstead of throwing so capture stays durable. ButloadMemoryConfig()hard-requiredEMBED_BASE_URL/EMBED_MODEL, so a host with a working pgvector Postgres and no embedding account couldn't construct the plane at all.EngineConfig.embed/MemoryConfigare now optional end to end:loadMemoryConfig()buildsembedonly when bothEMBED_BASE_URLandEMBED_MODELare set (trimmed, matchingrequireEnv's existing behavior — a whitespace-only value is treated as unset); leaving both unset opts into lexical-only. Setting exactly one throws — that's a real misconfiguration, not an opt-in.hybridSearch(src/services/search.ts) skips the dense channel entirely when there's no embed client config — no HTTP call is dispatched — instead of attempting-and-failing on every query.captureDocumentstill writes documents when embed is unconfigured: chunks are stored, vectors are not, and the embed-model registry (ensureEmbedModel/activateEmbedModel) is never reached.Degrade-flag decision
Reused the existing flags rather than inventing one, per the two states they already model:
dense_unavailable— existing meaning, "dense contributed nothing this call" (a configured endpoint that's down, or a tenant with no active model yet).lexical_only— was already reserved inSearchDegradedReasonSchema(src/core/schemas/search.ts) but never produced by any code path; it now means the structural "no embed endpoint configured on this engine at all," distinct from a transient failure.searchreportsdegraded: ["dense_unavailable", "lexical_only"]on every call when unconfigured.lexical_onlywas added toDegradeFlag(hybrid-search.ts) andDEGRADE_FLAG_SET(degrade-metrics.ts, compile-enforced viasatisfies). One side effect worth flagging: for a deliberately lexical-only host, this flag sits at a permanent ~100% windowed rate and will escalate tolog.errorindegrade-metrics.tsand stay there — accurate ("running degraded"), not a bug, but noisier than the module's other flags. Being filed as its own follow-up (config-state flags vs. failure flags need separating in that module), out of scope here.addmirrors this shape.add'sdegradedwas a bare boolean, which made it impossible for an integrator to write one "is this response degraded" check across both verbs. It's nowCaptureDegradedReason[]("embed_unavailable"/"lexical_only"), threaded throughCaptureResult→DocumentStoreAddResult→MemoryAddResult→ the/addHTTP route.embed_unavailableis capture's counterpart todense_unavailable(the embed pass ran and failed — client error, timeout, or a rejected/dims-mismatched chunk); paired withlexical_onlyspecifically when there's no endpoint configured at all.CaptureDegradedReasonlives incore/embed-worker.ts(notservices/capture.ts) soports/types.tscan reference it the same way it already referencesDegradeFlagforDocumentStoreSearchResult.Final wire shape, for a settings page coding against both:
Both are arrays, both omitted when healthy — one
Array.isArray(res.degraded) && res.degraded.length > 0check works for either.Migration note:
dense_unavailablenow also fires permanently for a deliberately lexical-only deployment, andadd'sdegradedis no longer a boolean — a host with an existing alert/check on either should check forlexical_onlyalongsidedense_unavailable, and switch anydegraded === truecheck to an array check. Noted inCHANGELOG.md.Capability discoverability
A host previously could only learn embeddings were unconfigured by running a search and inspecting degrade flags — meaning a lexical-only plane could run for months with nobody noticing recall was limited.
Memory.capabilities.embeddingsConfigured(on the handlecreateMemoryreturns) now answers this at construction time, no query needed:Backed by an optional
DocumentStore.capabilities(ports/types.ts) so a vendor store can report its own; a custom store that omits it defaults toembeddingsConfigured: true(the pre-CL-6287 assumption, since this SDK can't otherwise introspect a store it doesn't own). BothMemoryCapabilitiesandDocumentStoreCapabilitiesare exported fromindex.ts.Vocabulary
Settled on "embed endpoint" (what the config actually is) over "embedding account" everywhere it appeared (
config.ts,engine-client-config.ts,mount-config.ts,IMPLEMENTATION.md,capture.test.ts,CHANGELOG.md) —hybrid-search.ts/search.ts/transform.tsalready used it consistently.Not renaming
lexical_only→dense_unconfigured: that string was already reserved inSearchDegradedReasonSchemabefore this work, so it isn't ours to rename in passing, even though the naming argument (name a mode, not a failure) is reasonable.Opting into lexical-only
Unset both
EMBED_BASE_URLandEMBED_MODEL(or omit them entirely). No other config change needed —addand lexicalsearchwork as before,RERANK_BASE_URLis independent and can still be set.The replay/backfill pipeline (
runTransform,promoteGenerationinservices/transform.ts) still requires an embed endpoint, since re-deriving a corpus is inherently a re-embedding operation — it fails loudly and early with a clear message if run against an engine with none configured, rather than throwing deep insidebuildEmbedClientConfig.Backfill (explicitly out of scope)
Re-embedding documents captured while lexical-only, once an embed endpoint is later added, is not implemented here (per the issue). A follow-up would need:
embedChunks/activateEmbedModelagainst already-captured, already-chunked rows without re-runningadaptAndPlan/deriveVersionInTransaction(capture.ts's embed pass is currently only reachable from the post-transaction hook ofcaptureDocument/deriveFromRawCapture).transform_runtable's shape may be reusable, but it's currently scoped to full re-derivation, not a vectors-only backfill).Testing
bun run typecheckandbun test ./src: 401 → 417 tests, all passing. Coverage added across the review rounds:mount-config.test.ts: both-set-or-both-unset validation, whitespace-onlyEMBED_BASE_URLtreated as unset.capture.test.ts:toEmbedClientConfig(undefined)returnsundefined;embedInsertedChunksWithConfigreturns[]for no chunks and["embed_unavailable", "lexical_only"]when unconfigured, without touching the embed-model registry.search.test.ts:hybridSearchagainst a fakeDb/RawSqlwith embed unconfigured — returns lexical results, reports both degrade flags, and makes zero embed HTTP calls / zero embed-model-registry access.memory.test.ts:capabilities.embeddingsConfiguredistrue/falsefor engine construction with/without embed, and defaultstruefor a custom store that doesn't report it.transform.test.ts:runTransformandpromoteGeneration's embed-absent guards, against a fakeDb— both fail loudly with a clear message and never reach per-row derivation or the embed-model registry.CL-6287