Skip to content

Commit 9302f84

Browse files
committed
fix(embeddings): override stale block params instead of omitting them
Review round 3. The generic handler merges the params() result over the original inputs (`{ ...inputs, ...transformedParams }`), so omitting a key leaves the stale value in place. The previous round dropped an unsupported taskType or dimensions by omission, which was therefore a no-op through the executor path: a reduction or task type chosen for one model still reached the tool after a model switch. Rewrite each stale field to an explicit `undefined`, which does override in a spread. Same class of bug for `model` itself, which was forwarded whenever present without checking it belongs to the selected provider. Every provider's model dropdown shares the `model` id, so switching provider kept the previous provider's model and failed at the route as a mismatch. It now falls back to the provider's default unless the saved model actually belongs to it. Tests assert the merged result rather than the returned object, since the return shape alone cannot distinguish an omitted key from an overridden one — which is exactly why the previous fix looked correct and was not.
1 parent 05efae9 commit 9302f84

2 files changed

Lines changed: 95 additions & 18 deletions

File tree

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

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,67 @@ describe('Embeddings block', () => {
149149
* clears a stored subblock value when its `dependsOn` fields change, so a
150150
* reduction chosen for one model outlives a switch to another.
151151
*/
152+
/**
153+
* The generic handler merges this result over the original inputs
154+
* (`{ ...inputs, ...transformedParams }`), so omitting a stale key leaves the
155+
* old value in place. Only an explicit `undefined` overrides it — asserting
156+
* the merged result is what actually pins the behavior.
157+
*/
158+
describe('stale values are overridden, not merely omitted', () => {
159+
const mergeLikeExecutor = (inputs: Record<string, unknown>) => ({
160+
...inputs,
161+
...EmbeddingsBlock.tools.config?.params?.(inputs),
162+
})
163+
164+
it('overrides a dimension the selected model does not offer', () => {
165+
const merged = mergeLikeExecutor({
166+
provider: 'openai',
167+
model: 'text-embedding-3-small',
168+
input: 'hello',
169+
apiKey: 'k',
170+
dimensions: '3072',
171+
})
172+
173+
expect(merged.dimensions).toBeUndefined()
174+
})
175+
176+
it('overrides a task type the selected model does not offer', () => {
177+
const merged = mergeLikeExecutor({
178+
provider: 'openai',
179+
model: 'text-embedding-3-small',
180+
input: 'hello',
181+
apiKey: 'k',
182+
taskType: 'similarity',
183+
})
184+
185+
expect(merged.taskType).toBeUndefined()
186+
})
187+
188+
it('replaces a model left over from a previous provider', () => {
189+
// Selecting Cohere with OpenAI's model still stored must not reach the
190+
// route, which rejects it as a provider mismatch.
191+
const merged = mergeLikeExecutor({
192+
provider: 'cohere',
193+
model: 'text-embedding-3-large',
194+
input: 'hello',
195+
apiKey: 'k',
196+
})
197+
198+
expect(merged.model).toBe('embed-v4.0')
199+
})
200+
201+
it('keeps a model that does belong to the selected provider', () => {
202+
const merged = mergeLikeExecutor({
203+
provider: 'openai',
204+
model: 'text-embedding-3-large',
205+
input: 'hello',
206+
apiKey: 'k',
207+
})
208+
209+
expect(merged.model).toBe('text-embedding-3-large')
210+
})
211+
})
212+
152213
it('drops a dimension the newly selected model no longer offers', () => {
153214
const params = EmbeddingsBlock.tools.config?.params
154215

apps/sim/blocks/blocks/embeddings.ts

Lines changed: 34 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -273,38 +273,54 @@ export const EmbeddingsBlock: BlockConfig<EmbeddingsResponse> = {
273273
const provider = params.provider as EmbeddingCatalogProvider
274274
return TOOL_ID_BY_PROVIDER[provider] ?? TOOL_ID_BY_PROVIDER.openai
275275
},
276+
/**
277+
* Every per-provider dropdown shares one subblock id (`model`,
278+
* `taskType`, `dimensions`) and nothing clears a stored value when its
279+
* `dependsOn` fields change, so a choice made for one provider or model
280+
* outlives a switch away from it.
281+
*
282+
* Each stale field is therefore rewritten to an explicit `undefined`
283+
* rather than omitted. The generic handler merges this result over the
284+
* original inputs (`{ ...inputs, ...transformedParams }`), so an omitted
285+
* key leaves the stale value untouched — only an explicit `undefined`
286+
* overrides it.
287+
*/
276288
params: (params) => {
277289
const provider = (params.provider as EmbeddingCatalogProvider) || 'openai'
278290
if (!params.input) {
279291
throw new Error('Input text is required')
280292
}
281-
const model = params.model || DEFAULT_MODEL_BY_PROVIDER[provider]
293+
294+
/** A model saved under a previous provider must not survive the switch. */
295+
const savedModel = params.model as string | undefined
296+
const model =
297+
savedModel && EMBEDDING_MODELS[savedModel]?.provider === provider
298+
? savedModel
299+
: DEFAULT_MODEL_BY_PROVIDER[provider]
300+
282301
const info = EMBEDDING_MODELS[model]
283-
const dimensions =
302+
const requested =
284303
params.dimensions !== undefined && params.dimensions !== ''
285304
? Number(params.dimensions)
286305
: undefined
306+
const dimensions =
307+
requested !== undefined &&
308+
!Number.isNaN(requested) &&
309+
info?.supportedDimensions?.includes(requested)
310+
? requested
311+
: undefined
312+
const taskType =
313+
params.taskType &&
314+
info?.supportedTaskTypes?.includes(params.taskType as EmbeddingTaskType)
315+
? (params.taskType as EmbeddingTaskType)
316+
: undefined
287317

288318
return {
289319
apiKey: params.apiKey,
290320
input: params.input,
291321
model,
292-
/** Only send capabilities the selected model actually declares. */
293-
...(info?.supportedTaskTypes &&
294-
params.taskType &&
295-
info.supportedTaskTypes.includes(params.taskType as EmbeddingTaskType) && {
296-
taskType: params.taskType,
297-
}),
298-
/**
299-
* Every per-model Dimensions dropdown shares the `dimensions` id, and
300-
* switching models does not clear the stored value — so a reduction
301-
* picked for one model can outlive it. Drop anything the current model
302-
* no longer offers and fall back to its native size, rather than
303-
* sending a value the dropdown stopped presenting.
304-
*/
305-
...(dimensions !== undefined &&
306-
!Number.isNaN(dimensions) &&
307-
info?.supportedDimensions?.includes(dimensions) && { dimensions }),
322+
taskType,
323+
dimensions,
308324
}
309325
},
310326
},

0 commit comments

Comments
 (0)