Skip to content

Commit 8003402

Browse files
fix: capture path drops embed timeoutMs (#30)
Capture built its own EmbedClientConfig literal instead of using search.ts's EngineConfig-to-client-config mapping, so EMBED_TIMEOUT_MS never reached the capture path and it kept timing out at embed-client.ts's 10s default. Extract the mapping into a single shared engine-client-config.ts used by search.ts and capture.ts (and re-exported from both for existing imports), so every construction site stays in sync with EngineConfig.
1 parent 3b6ded2 commit 8003402

4 files changed

Lines changed: 115 additions & 65 deletions

File tree

‎src/core/engine-client-config.ts‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
/**
2+
* Maps `EngineConfig`'s embed/rerank sub-objects to the client configs
3+
* `embed-client.ts`/`rerank-client.ts` dispatch on. This is the ONLY place
4+
* that mapping happens — every construction site (search, capture, and any
5+
* future caller) must go through these functions so operator overrides like
6+
* EMBED_TIMEOUT_MS / RERANK_TIMEOUT_MS reach every code path uniformly.
7+
*/
8+
import type { EngineConfig } from "../config.ts";
9+
import type { EmbedClientConfig } from "./embed-client.ts";
10+
import type { RerankClientConfig } from "./rerank-client.ts";
11+
12+
const VALID_EMBED_API_STYLES = new Set(["openai", "tei", "ollama"]);
13+
14+
// The engine's `EngineConfig.embed.apiStyle` is a plain, operator-set string
15+
// (config.ts has no arktype gate on it); `EmbedClientConfig` requires the
16+
// literal union `embed-client.ts` dispatches on. Validated here, once, at
17+
// the trust boundary between config and the client — an invalid value is an
18+
// operator misconfiguration and must fail loudly, not silently degrade.
19+
// Built from the engine's own operator-configured embed endpoint — a trusted
20+
// URL, the same as KNOWLEDGE_DATABASE_URL.
21+
export function toEmbedClientConfig(
22+
embed: EngineConfig["embed"],
23+
): EmbedClientConfig {
24+
if (!VALID_EMBED_API_STYLES.has(embed.apiStyle)) {
25+
throw new Error(
26+
`Invalid EMBED_API_STYLE "${embed.apiStyle}" — must be one of: ${[...VALID_EMBED_API_STYLES].join(", ")}`,
27+
);
28+
}
29+
return {
30+
baseUrl: embed.baseUrl,
31+
modelId: embed.model,
32+
apiStyle: embed.apiStyle as EmbedClientConfig["apiStyle"],
33+
...(embed.apiKey !== undefined ? { apiKey: embed.apiKey } : {}),
34+
...(embed.timeoutMs !== undefined ? { timeoutMs: embed.timeoutMs } : {}),
35+
};
36+
}
37+
38+
// `EngineConfig.rerank` carries no `apiStyle` field — the engine currently
39+
// wires only a TEI-compatible cross-encoder endpoint (the locked default
40+
// model, `bge-reranker-v2-m3`, is TEI-servable); rerank apiStyle is hardcoded
41+
// `"tei"` below. Absent `baseUrl` => rerank is unconfigured => `undefined`,
42+
// same degrade-soft precedent as the embed config being absent upstream.
43+
// Built from the engine's own operator-configured rerank endpoint — a trusted
44+
// URL, the same as KNOWLEDGE_DATABASE_URL.
45+
export function toRerankClientConfig(
46+
rerank: EngineConfig["rerank"],
47+
): RerankClientConfig | undefined {
48+
if (!rerank.baseUrl) return undefined;
49+
return {
50+
baseUrl: rerank.baseUrl,
51+
apiStyle: "tei",
52+
...(rerank.model !== undefined ? { model: rerank.model } : {}),
53+
...(rerank.apiKey !== undefined ? { apiKey: rerank.apiKey } : {}),
54+
...(rerank.maxDocChars !== undefined
55+
? { maxDocChars: rerank.maxDocChars }
56+
: {}),
57+
...(rerank.timeoutMs !== undefined
58+
? { timeoutMs: rerank.timeoutMs }
59+
: {}),
60+
};
61+
}

‎src/services/capture.test.ts‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import { describe, expect, it } from "bun:test";
2+
import { toEmbedClientConfig } from "./capture.ts";
3+
import type { EngineConfig } from "../config.ts";
4+
5+
// Regression for the capture path silently timing out at embed-client.ts's
6+
// default (10000ms) even when EMBED_TIMEOUT_MS was set: capture.ts used to
7+
// build its own EmbedClientConfig literal instead of going through the same
8+
// mapping search.ts used, and dropped timeoutMs. capture.ts now re-exports
9+
// the one shared mapping (engine-client-config.ts) — assert it carries
10+
// EngineConfig.embed.timeoutMs through on the capture path specifically.
11+
describe("capture path embed client config", () => {
12+
it("carries EngineConfig.embed.timeoutMs through to the capture-path EmbedClientConfig", () => {
13+
const embed: EngineConfig["embed"] = {
14+
baseUrl: "http://embed.example",
15+
model: "test-model",
16+
apiStyle: "openai",
17+
apiKey: undefined,
18+
timeoutMs: 5000,
19+
};
20+
21+
const embedClientConfig = toEmbedClientConfig(embed);
22+
23+
expect(embedClientConfig.timeoutMs).toBe(5000);
24+
});
25+
26+
it("leaves timeoutMs undefined (so embed-client.ts's own default applies) when EngineConfig doesn't set one", () => {
27+
const embed: EngineConfig["embed"] = {
28+
baseUrl: "http://embed.example",
29+
model: "test-model",
30+
apiStyle: "openai",
31+
apiKey: undefined,
32+
timeoutMs: undefined,
33+
};
34+
35+
const embedClientConfig = toEmbedClientConfig(embed);
36+
37+
expect(embedClientConfig.timeoutMs).toBeUndefined();
38+
});
39+
});

‎src/services/capture.ts‎

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { type } from "arktype";
21
import { createHash } from "node:crypto";
32
import { and, desc, eq } from "drizzle-orm";
43
import type { Db, RawSql } from "../db/client.ts";
@@ -28,8 +27,9 @@ import type {
2827
import type { KnowledgeEdgeHint } from "../core/schemas/entity-edge.ts";
2928
import { createRawSqlClient } from "../core/embed-sql.ts";
3029
import { activateEmbedModel } from "../core/embed-model-registry.ts";
31-
import { EmbedClientConfigSchema, type EmbedClientConfig } from "../core/embed-client.ts";
30+
import type { EmbedClientConfig } from "../core/embed-client.ts";
3231
import { embedChunks, type EmbeddableChunk } from "../core/embed-worker.ts";
32+
import { toEmbedClientConfig } from "../core/engine-client-config.ts";
3333

3434
type Tx = Parameters<Parameters<Db["transaction"]>[0]>[0];
3535

@@ -462,21 +462,10 @@ async function captureInTransaction(
462462
);
463463
}
464464

465-
// Built from the engine's own operator-configured embed endpoint — a trusted
466-
// URL, the same as KNOWLEDGE_DATABASE_URL.
467-
function toEmbedClientConfig(embed: EngineConfig["embed"]): EmbedClientConfig {
468-
const candidate = {
469-
baseUrl: embed.baseUrl,
470-
modelId: embed.model,
471-
apiStyle: embed.apiStyle,
472-
...(embed.apiKey !== undefined ? { apiKey: embed.apiKey } : {}),
473-
};
474-
const parsed = EmbedClientConfigSchema(candidate);
475-
if (parsed instanceof type.errors) {
476-
throw new Error(`Invalid embed client config: ${parsed.summary}`);
477-
}
478-
return parsed;
479-
}
465+
// Re-exported so tests can assert the capture path resolves its embed
466+
// client config through the one shared mapping (see engine-client-config.ts)
467+
// rather than a capture-local duplicate that could drop fields like timeoutMs.
468+
export { toEmbedClientConfig };
480469

481470
// Embeds a version's freshly-inserted chunks and stores their vectors, after
482471
// the derivation transaction has already committed. Best-effort in the

‎src/services/search.ts‎

Lines changed: 9 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@ import {
1515
resolveActiveEmbedTable,
1616
} from "../core/embed-model-registry.ts";
1717
import { embedTexts, type EmbedClientConfig } from "../core/embed-client.ts";
18+
import {
19+
toEmbedClientConfig,
20+
toRerankClientConfig,
21+
} from "../core/engine-client-config.ts";
1822
import {
1923
rerankDocuments,
2024
RerankConfigError,
@@ -652,54 +656,11 @@ function applyBoosts(
652656
});
653657
}
654658

655-
const VALID_EMBED_API_STYLES = new Set(["openai", "tei", "ollama"]);
656-
657-
// The engine's `EngineConfig.embed.apiStyle` is a plain, operator-set string
658-
// (config.ts has no arktype gate on it); `EmbedClientConfig` requires the
659-
// literal union `embed-client.ts` dispatches on. Validated here, once, at
660-
// the trust boundary between config and the client — an invalid value is an
661-
// operator misconfiguration and must fail loudly, not silently degrade.
662-
// Built from the engine's own operator-configured embed endpoint — a trusted
663-
// URL, the same as KNOWLEDGE_DATABASE_URL.
664-
function toEmbedClientConfig(embed: EngineConfig["embed"]): EmbedClientConfig {
665-
if (!VALID_EMBED_API_STYLES.has(embed.apiStyle)) {
666-
throw new Error(
667-
`Invalid EMBED_API_STYLE "${embed.apiStyle}" — must be one of: ${[...VALID_EMBED_API_STYLES].join(", ")}`,
668-
);
669-
}
670-
return {
671-
baseUrl: embed.baseUrl,
672-
modelId: embed.model,
673-
apiStyle: embed.apiStyle as EmbedClientConfig["apiStyle"],
674-
...(embed.apiKey !== undefined ? { apiKey: embed.apiKey } : {}),
675-
...(embed.timeoutMs !== undefined ? { timeoutMs: embed.timeoutMs } : {}),
676-
};
677-
}
678-
679-
// `EngineConfig.rerank` carries no `apiStyle` field — the engine currently
680-
// wires only a TEI-compatible cross-encoder endpoint (the locked default
681-
// model, `bge-reranker-v2-m3`, is TEI-servable); rerank apiStyle is hardcoded
682-
// `"tei"` below. Absent `baseUrl` => rerank is unconfigured => `undefined`,
683-
// same degrade-soft precedent as the embed config being absent upstream.
684-
// Built from the engine's own operator-configured rerank endpoint — a trusted
685-
// URL, the same as KNOWLEDGE_DATABASE_URL.
686-
export function toRerankClientConfig(
687-
rerank: EngineConfig["rerank"],
688-
): RerankClientConfig | undefined {
689-
if (!rerank.baseUrl) return undefined;
690-
return {
691-
baseUrl: rerank.baseUrl,
692-
apiStyle: "tei",
693-
...(rerank.model !== undefined ? { model: rerank.model } : {}),
694-
...(rerank.apiKey !== undefined ? { apiKey: rerank.apiKey } : {}),
695-
...(rerank.maxDocChars !== undefined
696-
? { maxDocChars: rerank.maxDocChars }
697-
: {}),
698-
...(rerank.timeoutMs !== undefined
699-
? { timeoutMs: rerank.timeoutMs }
700-
: {}),
701-
};
702-
}
659+
// Single shared EngineConfig -> client-config mapping, used by every
660+
// construction site (search, capture) so operator overrides like
661+
// EMBED_TIMEOUT_MS reach every code path uniformly. Re-exported here since
662+
// this is the module memory.ts already imports `toRerankClientConfig` from.
663+
export { toEmbedClientConfig, toRerankClientConfig };
703664

704665
export interface HybridSearchDeps {
705666
db: Db;

0 commit comments

Comments
 (0)