Skip to content

Commit dd636b8

Browse files
committed
docs(embeddings): correct comments that drifted from the code
A comment pass over the feature found four that no longer matched what they sat on, all introduced by earlier rounds of this work. The contract's `satisfies` note promised that adding a catalog provider could not leave the wire enum stale. It cannot deliver that: `satisfies` proves every listed member is valid, not that the list is exhaustive, so an addition stays silently absent. Reworded to say what it does and does not catch. The client cited Gemini as a provider that omits usage, which the Gemini adapter now contradicts — it reads `usageMetadata.promptTokenCount`. Every adapter defines `parseTokens`, so the fallback is about a response lacking a usage block, not about a particular provider. `l2Normalize` documented only Gemini, though Cohere now calls it for a different and stronger reason, and "normalizes in place" read as mutation when the function returns a copy. The route's new size-guard comment claimed it avoids copying the payload; nothing there copies. The real reason is that summing lengths gates before the per-entry character scan. Also: split the derived-sub-block TSDoc so both constants carry hover text, gave the payload cap its own doc, dropped one comment that restated a signature, and tightened two long blocks without losing a fact.
1 parent 5d452da commit dd636b8

8 files changed

Lines changed: 39 additions & 26 deletions

File tree

apps/sim/app/api/tools/embeddings/route.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,10 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
8787
{ status: 400 }
8888
)
8989
}
90-
/** Size is checked before the per-entry scan so an oversized body is rejected without copying it. */
90+
/**
91+
* Summing lengths is cheap and runs before the per-entry whitespace scan, so
92+
* an oversized body is rejected without walking every character.
93+
*/
9194
const totalChars = texts.reduce((sum, text) => sum + text.length, 0)
9295
if (totalChars > MAX_EMBEDDING_TOTAL_CHARS) {
9396
return NextResponse.json(

apps/sim/blocks/blocks/embeddings.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,9 @@ const TASK_TYPE_LABELS: Record<EmbeddingTaskType, string> = {
3939
}
4040

4141
/**
42-
* Model, task-type, and dimension dropdowns are derived from the catalog rather
43-
* than hand-copied, so adding a catalog model cannot leave this block stale.
44-
* Each dropdown is scoped by a `condition` naming the provider (and model, where
45-
* the capability is per-model) because every variant shares one sub-block id.
42+
* Dropdowns are derived from the catalog rather than hand-copied, so adding a
43+
* catalog model cannot leave this block stale. Every variant shares one
44+
* sub-block id, so each is scoped by a `condition` naming the provider.
4645
*/
4746
const MODEL_SUB_BLOCKS: SubBlockConfig[] = EMBEDDING_CATALOG_PROVIDERS.map((provider) => ({
4847
id: 'model',
@@ -54,6 +53,11 @@ const MODEL_SUB_BLOCKS: SubBlockConfig[] = EMBEDDING_CATALOG_PROVIDERS.map((prov
5453
dependsOn: ['provider'],
5554
}))
5655

56+
/**
57+
* Task-type and dimension dropdowns, which are per-model rather than
58+
* per-provider: the `condition` names both, and a model contributes a dropdown
59+
* only for the capabilities the catalog says it has.
60+
*/
5761
const CAPABILITY_SUB_BLOCKS: SubBlockConfig[] = Object.entries(EMBEDDING_MODELS).flatMap(
5862
([model, info]) => {
5963
const scope = { field: 'provider', value: info.provider, and: { field: 'model', value: model } }

apps/sim/lib/api/contracts/tools/embeddings.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@ import { defineRouteContract } from '@/lib/api/contracts/types'
33
import type { EmbeddingCatalogProvider, EmbeddingTaskType } from '@/lib/embeddings/types'
44

55
/**
6-
* `satisfies` ties the wire enums to the catalog's own unions, so adding a
7-
* provider or task type there cannot silently leave the API contract stale.
6+
* `satisfies` ties the wire enums to the catalog's own unions: renaming or
7+
* removing a catalog member breaks the build here. It does NOT catch an
8+
* addition — a new catalog provider or task type stays absent from the wire
9+
* enum until it is added below.
810
*/
911
export const embeddingProviders = [
1012
'openai',
@@ -23,6 +25,7 @@ export const embeddingTaskTypes = [
2325

2426
/** Guards the route against unbounded fan-out into a paid provider. */
2527
export const MAX_EMBEDDING_INPUTS = 1000
28+
/** Caps total payload size independently of the input count. */
2629
export const MAX_EMBEDDING_TOTAL_CHARS = 1_000_000
2730

2831
const MISSING_EMBEDDING_FIELDS_ERROR = 'Missing required fields: provider, apiKey, and input'

apps/sim/lib/embeddings/catalog.ts

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -207,12 +207,11 @@ export function getKbEligibleModels(): string[] {
207207
* True when a model's tokens cannot be counted exactly.
208208
*
209209
* Batching measures with tiktoken, which only has encodings for OpenAI models —
210-
* every other id falls back to `cl100k_base`, so a Gemini, Cohere, or Mistral
211-
* ceiling is enforced in approximate units. The ceiling is still applied
212-
* exactly as declared: discounting it to absorb the error would truncate valid
213-
* content silently, which is worse than the alternative it guards against. An
214-
* undercount surfaces as a provider rejection, which is visible and
215-
* actionable; silently shortening an embedding's input is not.
210+
* every other id falls back to `cl100k_base`, so Gemini, Cohere, and Mistral
211+
* ceilings are enforced in approximate units. The ceiling is still applied as
212+
* declared rather than discounted to absorb the error: an undercount surfaces
213+
* as a visible provider rejection, whereas silently shortening an embedding's
214+
* input does not.
216215
*/
217216
export function hasApproximateTokenCount(info: EmbeddingModelInfo): boolean {
218217
return info.tokenizerProvider !== 'openai'

apps/sim/lib/embeddings/client.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -157,9 +157,13 @@ async function callEmbeddingAPI(
157157

158158
const json = await response.json()
159159
const embeddings = request.parse(json)
160+
/**
161+
* Fallback for a response that carries no usage block. Estimated with the
162+
* provider's own tokenizer, which is approximate for every non-OpenAI
163+
* model — see {@link hasApproximateTokenCount}.
164+
*/
160165
const totalTokens =
161166
request.parseTokens?.(json) ??
162-
// Providers that omit usage (e.g. Gemini) get an estimate from their tokenizer
163167
inputs.reduce(
164168
(sum, text) => sum + estimateTokenCount(text, provider.info.tokenizerProvider).count,
165169
0
@@ -232,10 +236,8 @@ export async function embed(texts: string[], options: EmbedOptions): Promise<Emb
232236
*
233237
* Three bounds compose here:
234238
*
235-
* 1. {@link BATCH_TOKEN_TARGET} is what we actually aim for. It is an
236-
* operational choice, not a provider limit: it keeps a single request well
237-
* inside {@link EMBEDDING_REQUEST_TIMEOUT_MS}, so a timeout costs one small
238-
* batch and its retries rather than a large one.
239+
* 1. {@link BATCH_TOKEN_TARGET} is what we actually aim for — an operational
240+
* choice, not a provider limit (see its declaration for the reasoning).
239241
* 2. A provider's documented summed-token cap, when it publishes one, is a
240242
* hard ceiling the target can never exceed.
241243
* 3. The per-input ceiling is a floor. A budget below it would make

apps/sim/lib/embeddings/normalize.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,13 @@
11
/**
2-
* L2-normalizes a vector in place of the provider doing it.
2+
* Returns an L2-normalized copy of a vector, for providers that do not
3+
* normalize a Matryoshka-reduced output themselves.
34
*
4-
* Gemini does NOT auto-normalize embeddings when `outputDimensionality` is set
5-
* below the native 3072 dimension on `gemini-embedding-001`. Normalizing
6-
* manually keeps cosine and inner-product similarity correct.
5+
* Gemini does NOT auto-normalize when `outputDimensionality` is set below the
6+
* native 3072 dimension on `gemini-embedding-001`. Cohere never documents
7+
* whether it renormalizes a truncated vector, so its reduced output is
8+
* normalized here too — the operation is idempotent, so it is a no-op on
9+
* already-unit vectors and a correctness fix otherwise. Both keep cosine and
10+
* inner-product similarity correct.
711
*/
812
export function l2Normalize(vector: number[]): number[] {
913
let sumSquares = 0

apps/sim/lib/knowledge/embeddings.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,9 +82,6 @@ export async function generateEmbeddings(
8282
}
8383
}
8484

85-
/**
86-
* Generate embedding for a single search query.
87-
*/
8885
export async function generateSearchEmbedding(
8986
query: string,
9087
embeddingModel: string = DEFAULT_EMBEDDING_MODEL,

apps/sim/tools/openai/embeddings.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@ import type { ToolConfig } from '@/tools/types'
66
* Legacy tool id retained for the sunset `openai` Embeddings block and for
77
* copilot/VFS callers that reference it by name. It is an alias of
88
* `embeddings_openai` so both ids execute the exact same code path; the output
9-
* shape only gains fields (`provider`, `dimensions`) relative to the original.
9+
* shape only gains fields (`provider`, `dimensions`, and the internal
10+
* `__embeddingTokens`) relative to the original.
1011
*/
1112
export const embeddingsTool: ToolConfig<EmbeddingsParams, EmbeddingsResponse> = {
1213
...embeddingsOpenAITool,

0 commit comments

Comments
 (0)