Skip to content

Commit 5d452da

Browse files
committed
refactor(embeddings): derive block options from the catalog; use shared helpers
Findings from a four-angle quality review. Reuse: `splitByItemLimit` and `processWithConcurrency` were reimplementations of `chunkArray` (`@sim/utils`) and `mapWithConcurrency` (`@/lib/core/utils/concurrency`), so `lib/embeddings/batching.ts` is gone. That helper's doc forbade a throwing mapper; embedding legitimately wants a failed batch to fail the call, since a partial vector set is not a usable result, so the contract is reworded to cover both intents rather than forked. The block no longer hand-copies the catalog. Its model, task-type, and dimension dropdowns are derived from `EMBEDDING_MODELS`, which deletes roughly 150 lines of literals that had to be kept in step by a drift test. The comment claiming this was impossible was wrong: `generate-docs.ts` only reads `subBlocks` looking for an `id: 'operation'` entry, which this block does not have. Verified by regenerating — `embeddings.mdx` and `integrations.json` come out byte-identical. Single-sourced two maps that were stated twice: BYOK provider ids (which encode the non-obvious gemini -> google mapping) and the per-provider default model. The route previously took its default from `getModelsForProvider(provider)[0]`, which silently depended on catalog key order. Azure's `endpoint` and `apiVersion` are required on their own context type instead of optional on the shared one, so the adapter can no longer be built without them and emit an `undefined/...` URL. Also: contract enums now `satisfies` the catalog unions so they cannot drift, the barrel exports only what callers outside the module use, the redundant `requestedDimensions` field is a parameter, the bare `getEmbeddingModelInfo()` call is a named `assertKbEmbeddingModel`, and the route checks payload size before scanning entries rather than copying the body first.
1 parent 26f1807 commit 5d452da

21 files changed

Lines changed: 237 additions & 352 deletions

File tree

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

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,23 +10,26 @@ import { getValidationErrorMessage, parseRequest, validationErrorResponse } from
1010
import { checkInternalAuth } from '@/lib/auth/hybrid'
1111
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1212
import {
13+
DEFAULT_MODEL_BY_PROVIDER,
1314
embed,
1415
findEmbeddingModelInfo,
15-
getModelsForProvider,
1616
resolveDimensions,
1717
} from '@/lib/embeddings'
1818

1919
const logger = createLogger('EmbeddingsToolAPI')
2020

2121
export const dynamic = 'force-dynamic'
2222

23-
/** Accepts a single string, an array, or a JSON-encoded array from a reference expression. */
23+
/**
24+
* Accepts a single string, an array, or a JSON-encoded array from a reference
25+
* expression. Probes for the opening bracket with a regex rather than `trim()`,
26+
* which would copy the whole payload just to read one character.
27+
*/
2428
function normalizeInput(input: string | string[]): string[] {
2529
if (Array.isArray(input)) return input
26-
const trimmed = input.trim()
27-
if (trimmed.startsWith('[')) {
30+
if (/^\s*\[/.test(input)) {
2831
try {
29-
const parsed = JSON.parse(trimmed)
32+
const parsed = JSON.parse(input)
3033
if (Array.isArray(parsed) && parsed.every((entry) => typeof entry === 'string')) {
3134
return parsed
3235
}
@@ -84,13 +87,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
8487
{ status: 400 }
8588
)
8689
}
87-
if (texts.some((text) => text.trim().length === 0)) {
88-
return NextResponse.json(
89-
{ success: false, error: 'input entries cannot be empty' },
90-
{ status: 400 }
91-
)
92-
}
93-
90+
/** Size is checked before the per-entry scan so an oversized body is rejected without copying it. */
9491
const totalChars = texts.reduce((sum, text) => sum + text.length, 0)
9592
if (totalChars > MAX_EMBEDDING_TOTAL_CHARS) {
9693
return NextResponse.json(
@@ -102,7 +99,14 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
10299
)
103100
}
104101

105-
const resolvedModel = model || getModelsForProvider(provider)[0]
102+
if (texts.some((text) => !/\S/.test(text))) {
103+
return NextResponse.json(
104+
{ success: false, error: 'input entries cannot be empty' },
105+
{ status: 400 }
106+
)
107+
}
108+
109+
const resolvedModel = model || DEFAULT_MODEL_BY_PROVIDER[provider]
106110
const info = findEmbeddingModelInfo(resolvedModel)
107111
if (!info) {
108112
return NextResponse.json(

apps/sim/blocks/blocks/embeddings.test.ts

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,14 @@
22
* @vitest-environment node
33
*/
44
import { describe, expect, it } from 'vitest'
5-
import { EMBEDDING_MODELS } from '@/lib/embeddings/catalog'
6-
import {
7-
DEFAULT_MODEL_BY_PROVIDER,
8-
EmbeddingsBlock,
9-
TOOL_ID_BY_PROVIDER,
10-
} from '@/blocks/blocks/embeddings'
5+
import { DEFAULT_MODEL_BY_PROVIDER, EMBEDDING_MODELS } from '@/lib/embeddings/catalog'
6+
import { EmbeddingsBlock, TOOL_ID_BY_PROVIDER } from '@/blocks/blocks/embeddings'
117

128
/**
13-
* The block spells its model, task-type, and dimension options out as literals
14-
* because `scripts/generate-docs.ts` parses the block file as source text and
15-
* cannot see computed values. These tests are what stop those literals from
16-
* drifting away from the catalog that actually drives the runtime.
9+
* The block derives its model, task-type, and dimension options from the
10+
* catalog, so these assert the derivation still produces what the UI expects:
11+
* one dropdown per provider/model, the catalog's own option sets, and the
12+
* native size pre-selected. They also pin the provider-to-tool routing.
1713
*/
1814

1915
function subBlocksById(id: string) {

0 commit comments

Comments
 (0)