diff --git a/src/core/engine-client-config.ts b/src/core/engine-client-config.ts new file mode 100644 index 0000000..cd48c5e --- /dev/null +++ b/src/core/engine-client-config.ts @@ -0,0 +1,61 @@ +/** + * Maps `EngineConfig`'s embed/rerank sub-objects to the client configs + * `embed-client.ts`/`rerank-client.ts` dispatch on. This is the ONLY place + * that mapping happens — every construction site (search, capture, and any + * future caller) must go through these functions so operator overrides like + * EMBED_TIMEOUT_MS / RERANK_TIMEOUT_MS reach every code path uniformly. + */ +import type { EngineConfig } from "../config.ts"; +import type { EmbedClientConfig } from "./embed-client.ts"; +import type { RerankClientConfig } from "./rerank-client.ts"; + +const VALID_EMBED_API_STYLES = new Set(["openai", "tei", "ollama"]); + +// The engine's `EngineConfig.embed.apiStyle` is a plain, operator-set string +// (config.ts has no arktype gate on it); `EmbedClientConfig` requires the +// literal union `embed-client.ts` dispatches on. Validated here, once, at +// the trust boundary between config and the client — an invalid value is an +// operator misconfiguration and must fail loudly, not silently degrade. +// Built from the engine's own operator-configured embed endpoint — a trusted +// URL, the same as KNOWLEDGE_DATABASE_URL. +export function toEmbedClientConfig( + embed: EngineConfig["embed"], +): EmbedClientConfig { + if (!VALID_EMBED_API_STYLES.has(embed.apiStyle)) { + throw new Error( + `Invalid EMBED_API_STYLE "${embed.apiStyle}" — must be one of: ${[...VALID_EMBED_API_STYLES].join(", ")}`, + ); + } + return { + baseUrl: embed.baseUrl, + modelId: embed.model, + apiStyle: embed.apiStyle as EmbedClientConfig["apiStyle"], + ...(embed.apiKey !== undefined ? { apiKey: embed.apiKey } : {}), + ...(embed.timeoutMs !== undefined ? { timeoutMs: embed.timeoutMs } : {}), + }; +} + +// `EngineConfig.rerank` carries no `apiStyle` field — the engine currently +// wires only a TEI-compatible cross-encoder endpoint (the locked default +// model, `bge-reranker-v2-m3`, is TEI-servable); rerank apiStyle is hardcoded +// `"tei"` below. Absent `baseUrl` => rerank is unconfigured => `undefined`, +// same degrade-soft precedent as the embed config being absent upstream. +// Built from the engine's own operator-configured rerank endpoint — a trusted +// URL, the same as KNOWLEDGE_DATABASE_URL. +export function toRerankClientConfig( + rerank: EngineConfig["rerank"], +): RerankClientConfig | undefined { + if (!rerank.baseUrl) return undefined; + return { + baseUrl: rerank.baseUrl, + apiStyle: "tei", + ...(rerank.model !== undefined ? { model: rerank.model } : {}), + ...(rerank.apiKey !== undefined ? { apiKey: rerank.apiKey } : {}), + ...(rerank.maxDocChars !== undefined + ? { maxDocChars: rerank.maxDocChars } + : {}), + ...(rerank.timeoutMs !== undefined + ? { timeoutMs: rerank.timeoutMs } + : {}), + }; +} diff --git a/src/services/capture.test.ts b/src/services/capture.test.ts new file mode 100644 index 0000000..6db8267 --- /dev/null +++ b/src/services/capture.test.ts @@ -0,0 +1,39 @@ +import { describe, expect, it } from "bun:test"; +import { toEmbedClientConfig } from "./capture.ts"; +import type { EngineConfig } from "../config.ts"; + +// Regression for the capture path silently timing out at embed-client.ts's +// default (10000ms) even when EMBED_TIMEOUT_MS was set: capture.ts used to +// build its own EmbedClientConfig literal instead of going through the same +// mapping search.ts used, and dropped timeoutMs. capture.ts now re-exports +// the one shared mapping (engine-client-config.ts) — assert it carries +// EngineConfig.embed.timeoutMs through on the capture path specifically. +describe("capture path embed client config", () => { + it("carries EngineConfig.embed.timeoutMs through to the capture-path EmbedClientConfig", () => { + const embed: EngineConfig["embed"] = { + baseUrl: "http://embed.example", + model: "test-model", + apiStyle: "openai", + apiKey: undefined, + timeoutMs: 5000, + }; + + const embedClientConfig = toEmbedClientConfig(embed); + + expect(embedClientConfig.timeoutMs).toBe(5000); + }); + + it("leaves timeoutMs undefined (so embed-client.ts's own default applies) when EngineConfig doesn't set one", () => { + const embed: EngineConfig["embed"] = { + baseUrl: "http://embed.example", + model: "test-model", + apiStyle: "openai", + apiKey: undefined, + timeoutMs: undefined, + }; + + const embedClientConfig = toEmbedClientConfig(embed); + + expect(embedClientConfig.timeoutMs).toBeUndefined(); + }); +}); diff --git a/src/services/capture.ts b/src/services/capture.ts index ca5fde6..2cfd24b 100644 --- a/src/services/capture.ts +++ b/src/services/capture.ts @@ -1,4 +1,3 @@ -import { type } from "arktype"; import { createHash } from "node:crypto"; import { and, desc, eq } from "drizzle-orm"; import type { Db, RawSql } from "../db/client.ts"; @@ -28,8 +27,9 @@ import type { import type { KnowledgeEdgeHint } from "../core/schemas/entity-edge.ts"; import { createRawSqlClient } from "../core/embed-sql.ts"; import { activateEmbedModel } from "../core/embed-model-registry.ts"; -import { EmbedClientConfigSchema, type EmbedClientConfig } from "../core/embed-client.ts"; +import type { EmbedClientConfig } from "../core/embed-client.ts"; import { embedChunks, type EmbeddableChunk } from "../core/embed-worker.ts"; +import { toEmbedClientConfig } from "../core/engine-client-config.ts"; type Tx = Parameters[0]>[0]; @@ -462,21 +462,10 @@ async function captureInTransaction( ); } -// Built from the engine's own operator-configured embed endpoint — a trusted -// URL, the same as KNOWLEDGE_DATABASE_URL. -function toEmbedClientConfig(embed: EngineConfig["embed"]): EmbedClientConfig { - const candidate = { - baseUrl: embed.baseUrl, - modelId: embed.model, - apiStyle: embed.apiStyle, - ...(embed.apiKey !== undefined ? { apiKey: embed.apiKey } : {}), - }; - const parsed = EmbedClientConfigSchema(candidate); - if (parsed instanceof type.errors) { - throw new Error(`Invalid embed client config: ${parsed.summary}`); - } - return parsed; -} +// Re-exported so tests can assert the capture path resolves its embed +// client config through the one shared mapping (see engine-client-config.ts) +// rather than a capture-local duplicate that could drop fields like timeoutMs. +export { toEmbedClientConfig }; // Embeds a version's freshly-inserted chunks and stores their vectors, after // the derivation transaction has already committed. Best-effort in the diff --git a/src/services/search.ts b/src/services/search.ts index 9205c81..419119c 100644 --- a/src/services/search.ts +++ b/src/services/search.ts @@ -15,6 +15,10 @@ import { resolveActiveEmbedTable, } from "../core/embed-model-registry.ts"; import { embedTexts, type EmbedClientConfig } from "../core/embed-client.ts"; +import { + toEmbedClientConfig, + toRerankClientConfig, +} from "../core/engine-client-config.ts"; import { rerankDocuments, RerankConfigError, @@ -652,54 +656,11 @@ function applyBoosts( }); } -const VALID_EMBED_API_STYLES = new Set(["openai", "tei", "ollama"]); - -// The engine's `EngineConfig.embed.apiStyle` is a plain, operator-set string -// (config.ts has no arktype gate on it); `EmbedClientConfig` requires the -// literal union `embed-client.ts` dispatches on. Validated here, once, at -// the trust boundary between config and the client — an invalid value is an -// operator misconfiguration and must fail loudly, not silently degrade. -// Built from the engine's own operator-configured embed endpoint — a trusted -// URL, the same as KNOWLEDGE_DATABASE_URL. -function toEmbedClientConfig(embed: EngineConfig["embed"]): EmbedClientConfig { - if (!VALID_EMBED_API_STYLES.has(embed.apiStyle)) { - throw new Error( - `Invalid EMBED_API_STYLE "${embed.apiStyle}" — must be one of: ${[...VALID_EMBED_API_STYLES].join(", ")}`, - ); - } - return { - baseUrl: embed.baseUrl, - modelId: embed.model, - apiStyle: embed.apiStyle as EmbedClientConfig["apiStyle"], - ...(embed.apiKey !== undefined ? { apiKey: embed.apiKey } : {}), - ...(embed.timeoutMs !== undefined ? { timeoutMs: embed.timeoutMs } : {}), - }; -} - -// `EngineConfig.rerank` carries no `apiStyle` field — the engine currently -// wires only a TEI-compatible cross-encoder endpoint (the locked default -// model, `bge-reranker-v2-m3`, is TEI-servable); rerank apiStyle is hardcoded -// `"tei"` below. Absent `baseUrl` => rerank is unconfigured => `undefined`, -// same degrade-soft precedent as the embed config being absent upstream. -// Built from the engine's own operator-configured rerank endpoint — a trusted -// URL, the same as KNOWLEDGE_DATABASE_URL. -export function toRerankClientConfig( - rerank: EngineConfig["rerank"], -): RerankClientConfig | undefined { - if (!rerank.baseUrl) return undefined; - return { - baseUrl: rerank.baseUrl, - apiStyle: "tei", - ...(rerank.model !== undefined ? { model: rerank.model } : {}), - ...(rerank.apiKey !== undefined ? { apiKey: rerank.apiKey } : {}), - ...(rerank.maxDocChars !== undefined - ? { maxDocChars: rerank.maxDocChars } - : {}), - ...(rerank.timeoutMs !== undefined - ? { timeoutMs: rerank.timeoutMs } - : {}), - }; -} +// Single shared EngineConfig -> client-config mapping, used by every +// construction site (search, capture) so operator overrides like +// EMBED_TIMEOUT_MS reach every code path uniformly. Re-exported here since +// this is the module memory.ts already imports `toRerankClientConfig` from. +export { toEmbedClientConfig, toRerankClientConfig }; export interface HybridSearchDeps { db: Db;