fix: capture path drops embed timeoutMs - #30
Merged
Conversation
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.
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
1 participant
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.
Bug
PR #29 (3b6ded2) plumbed
EMBED_TIMEOUT_MSthroughEngineConfigand intosrc/services/search.ts'stoEmbedClientConfig. ButcaptureDocument(src/services/capture.ts) never called that function — it built its ownEmbedClientConfigobject literal (validated viaEmbedClientConfigSchemadirectly) that omittedtimeoutMsentirely. So in production, capture kept hitting embed-client.ts's 10000ms default regardless of the env var.Construction sites found
src/services/search.ts—toEmbedClientConfig/toRerankClientConfig: includedtimeoutMs.src/services/capture.ts— capture-localtoEmbedClientConfig: missingtimeoutMs(the bug).src/core/embed-worker.ts— takesEmbedClientConfigas a parameter only, doesn't construct one.src/services/transform.ts—buildEmbedClientConfig/buildRerankClientConfig: build per-request derivation configs from replay params, not fromEngineConfig; out of scope (different inputs, not part of this duplication).EngineConfig-based mapping — onlysearch.tsbuilt one, and capture never needed rerank.Fix
Extracted
toEmbedClientConfig/toRerankClientConfiginto a single shared module,src/core/engine-client-config.ts. Bothsearch.tsandcapture.tsnow import and re-export from there instead of each defining/duplicating the mapping.capture.ts's capture-local literal + schema validation is gone.Test
Added
src/services/capture.test.tsasserting the capture path's embed client config (viatoEmbedClientConfigas re-exported bycapture.ts) carriesEngineConfig.embed.timeoutMsthrough, plus the undefined-default case.bun test ./src— 320 pass, 0 fail.