Skip to content

Commit 319e0db

Browse files
improvement(mothership): agent model dropdown validations, markers for recommended models (#4213)
* improvement(mothership): agent model dropdown validations, recommendation system * mark a few more models: * remove regex based checks' * remove dead code * remove inherited reseller flags * fix note * address bugbot comments * code cleanup
1 parent 003e931 commit 319e0db

File tree

4 files changed

+240
-13
lines changed

4 files changed

+240
-13
lines changed

apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.test.ts

Lines changed: 128 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,9 @@
44
import { beforeEach, describe, expect, it, vi } from 'vitest'
55
import { normalizeConditionRouterIds } from './builders'
66

7-
const { mockValidateSelectorIds } = vi.hoisted(() => ({
7+
const { mockValidateSelectorIds, mockGetModelOptions } = vi.hoisted(() => ({
88
mockValidateSelectorIds: vi.fn(),
9+
mockGetModelOptions: vi.fn(() => []),
910
}))
1011

1112
const conditionBlockConfig = {
@@ -26,7 +27,24 @@ const routerBlockConfig = {
2627
type: 'router_v2',
2728
name: 'Router',
2829
outputs: {},
29-
subBlocks: [{ id: 'routes', type: 'router-input' }],
30+
subBlocks: [
31+
{ id: 'routes', type: 'router-input' },
32+
{ id: 'model', type: 'combobox', options: mockGetModelOptions },
33+
],
34+
}
35+
36+
const agentBlockConfig = {
37+
type: 'agent',
38+
name: 'Agent',
39+
outputs: {},
40+
subBlocks: [{ id: 'model', type: 'combobox', options: mockGetModelOptions }],
41+
}
42+
43+
const huggingfaceBlockConfig = {
44+
type: 'huggingface',
45+
name: 'HuggingFace',
46+
outputs: {},
47+
subBlocks: [{ id: 'model', type: 'short-input' }],
3048
}
3149

3250
vi.mock('@/blocks/registry', () => ({
@@ -37,7 +55,15 @@ vi.mock('@/blocks/registry', () => ({
3755
? oauthBlockConfig
3856
: type === 'router_v2'
3957
? routerBlockConfig
40-
: undefined,
58+
: type === 'agent'
59+
? agentBlockConfig
60+
: type === 'huggingface'
61+
? huggingfaceBlockConfig
62+
: undefined,
63+
}))
64+
65+
vi.mock('@/blocks/utils', () => ({
66+
getModelOptions: mockGetModelOptions,
4167
}))
4268

4369
vi.mock('@/lib/copilot/validation/selector-validator', () => ({
@@ -83,6 +109,105 @@ describe('validateInputsForBlock', () => {
83109
expect(result.errors).toHaveLength(1)
84110
expect(result.errors[0]?.error).toContain('expected a JSON array')
85111
})
112+
113+
it('accepts known agent model ids', () => {
114+
const result = validateInputsForBlock('agent', { model: 'claude-sonnet-4-6' }, 'agent-1')
115+
116+
expect(result.errors).toHaveLength(0)
117+
expect(result.validInputs.model).toBe('claude-sonnet-4-6')
118+
})
119+
120+
it('rejects hallucinated agent model ids that match a static provider pattern', () => {
121+
const result = validateInputsForBlock('agent', { model: 'claude-sonnet-4.6' }, 'agent-1')
122+
123+
expect(result.validInputs.model).toBeUndefined()
124+
expect(result.errors).toHaveLength(1)
125+
expect(result.errors[0]?.field).toBe('model')
126+
expect(result.errors[0]?.error).toContain('Unknown model id')
127+
expect(result.errors[0]?.error).toContain('claude-sonnet-4-6')
128+
})
129+
130+
it('rejects legacy claude-4.5-haiku style ids', () => {
131+
const result = validateInputsForBlock('agent', { model: 'claude-4.5-haiku' }, 'agent-1')
132+
133+
expect(result.errors).toHaveLength(1)
134+
expect(result.errors[0]?.error).toContain('Unknown model id')
135+
})
136+
137+
it('allows empty model values', () => {
138+
const result = validateInputsForBlock('agent', { model: '' }, 'agent-1')
139+
140+
expect(result.errors).toHaveLength(0)
141+
expect(result.validInputs.model).toBe('')
142+
})
143+
144+
it('allows custom ollama-prefixed model ids', () => {
145+
const result = validateInputsForBlock('agent', { model: 'ollama/my-private-model' }, 'agent-1')
146+
147+
expect(result.errors).toHaveLength(0)
148+
expect(result.validInputs.model).toBe('ollama/my-private-model')
149+
})
150+
151+
it('validates the model field on router_v2 blocks too', () => {
152+
const valid = validateInputsForBlock('router_v2', { model: 'claude-sonnet-4-6' }, 'router-1')
153+
expect(valid.errors).toHaveLength(0)
154+
expect(valid.validInputs.model).toBe('claude-sonnet-4-6')
155+
156+
const invalid = validateInputsForBlock('router_v2', { model: 'claude-sonnet-4.6' }, 'router-1')
157+
expect(invalid.validInputs.model).toBeUndefined()
158+
expect(invalid.errors).toHaveLength(1)
159+
expect(invalid.errors[0]?.blockType).toBe('router_v2')
160+
expect(invalid.errors[0]?.field).toBe('model')
161+
expect(invalid.errors[0]?.error).toContain('Unknown model id')
162+
})
163+
164+
it("does not apply model validation to blocks whose model field is not Sim's catalog", () => {
165+
const result = validateInputsForBlock(
166+
'huggingface',
167+
{ model: 'mistralai/Mistral-7B-Instruct-v0.3' },
168+
'hf-1'
169+
)
170+
171+
expect(result.errors).toHaveLength(0)
172+
expect(result.validInputs.model).toBe('mistralai/Mistral-7B-Instruct-v0.3')
173+
})
174+
175+
it('rejects a bare Ollama-style tag without the provider prefix', () => {
176+
const result = validateInputsForBlock('agent', { model: 'llama3.1:8b' }, 'agent-1')
177+
178+
expect(result.validInputs.model).toBeUndefined()
179+
expect(result.errors).toHaveLength(1)
180+
expect(result.errors[0]?.error).toContain('Unknown model id')
181+
expect(result.errors[0]?.error).toContain('ollama/')
182+
})
183+
184+
it('rejects date-pinned ids that are not literally in the catalog', () => {
185+
const result = validateInputsForBlock(
186+
'agent',
187+
{ model: 'claude-sonnet-4-5-20250929' },
188+
'agent-1'
189+
)
190+
191+
expect(result.validInputs.model).toBeUndefined()
192+
expect(result.errors).toHaveLength(1)
193+
expect(result.errors[0]?.error).toContain('Unknown model id')
194+
})
195+
196+
it('trims whitespace around catalog model ids and stores the trimmed value', () => {
197+
const result = validateInputsForBlock('agent', { model: ' gpt-5.4 ' }, 'agent-1')
198+
199+
expect(result.errors).toHaveLength(0)
200+
expect(result.validInputs.model).toBe('gpt-5.4')
201+
})
202+
203+
it('rejects a pattern-matching but uncataloged id even with surrounding whitespace', () => {
204+
const result = validateInputsForBlock('agent', { model: ' gpt-100-ultra ' }, 'agent-1')
205+
206+
expect(result.validInputs.model).toBeUndefined()
207+
expect(result.errors).toHaveLength(1)
208+
expect(result.errors[0]?.error).toContain('gpt-100-ultra')
209+
expect(result.errors[0]?.error).not.toMatch(/\s{2,}/)
210+
})
86211
})
87212

88213
describe('normalizeConditionRouterIds', () => {

apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.ts

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@ import { validateSelectorIds } from '@/lib/copilot/validation/selector-validator
33
import type { PermissionGroupConfig } from '@/lib/permission-groups/types'
44
import { getBlock } from '@/blocks/registry'
55
import type { SubBlockConfig } from '@/blocks/types'
6+
import { getModelOptions } from '@/blocks/utils'
67
import { EDGE, normalizeName } from '@/executor/constants'
8+
import { isKnownModelId, suggestModelIdsForUnknownModel } from '@/providers/models'
79
import { TRIGGER_RUNTIME_SUBBLOCK_IDS } from '@/triggers/constants'
810
import type {
911
EdgeHandleValidationResult,
@@ -350,9 +352,31 @@ export function validateValueForSubBlockType(
350352
case 'short-input':
351353
case 'long-input':
352354
case 'combobox': {
353-
// Should be string (combobox allows custom values)
355+
const usesProviderCatalog =
356+
fieldName === 'model' && subBlockConfig.options === getModelOptions
357+
358+
if (usesProviderCatalog) {
359+
const stringValue = typeof value === 'string' ? value : String(value)
360+
const trimmed = stringValue.trim()
361+
if (trimmed !== '' && !isKnownModelId(trimmed)) {
362+
const suggestions = suggestModelIdsForUnknownModel(trimmed)
363+
const suggestionText =
364+
suggestions.length > 0 ? ` Valid options include: ${suggestions.join(', ')}.` : ''
365+
return {
366+
valid: false,
367+
error: {
368+
blockId,
369+
blockType,
370+
field: fieldName,
371+
value,
372+
error: `Unknown model id "${trimmed}" for block "${blockType}". Read components/blocks/${blockType}.json (the model.options array) for valid ids; prefer entries with recommended: true and avoid deprecated: true. For user-configured models (Ollama, vLLM, OpenRouter, Fireworks), prefix the id with the provider slash, e.g. "ollama/llama3.1:8b".${suggestionText}`,
373+
},
374+
}
375+
}
376+
return { valid: true, value: trimmed }
377+
}
378+
354379
if (typeof value !== 'string' && typeof value !== 'number') {
355-
// Convert to string but don't error
356380
return { valid: true, value: String(value) }
357381
}
358382
return { valid: true, value }

apps/sim/lib/copilot/vfs/serializers.ts

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { getCopilotToolDescription } from '@/lib/copilot/tools/descriptions'
22
import { isHosted } from '@/lib/core/config/feature-flags'
33
import { isSubBlockHidden } from '@/lib/workflows/subblocks/visibility'
44
import type { BlockConfig, SubBlockConfig } from '@/blocks/types'
5-
import { PROVIDER_DEFINITIONS } from '@/providers/models'
5+
import { DYNAMIC_MODEL_PROVIDERS, PROVIDER_DEFINITIONS } from '@/providers/models'
66
import type { ToolConfig } from '@/tools/types'
77

88
/**
@@ -320,24 +320,38 @@ export function serializeTableMeta(table: {
320320
* Excludes dynamic providers (ollama, vllm, openrouter) whose models are user-configured.
321321
* Includes provider ID and whether the model is hosted by Sim (no API key required).
322322
*/
323-
function getStaticModelOptionsForVFS(): Array<{
323+
interface StaticModelOption {
324324
id: string
325325
provider: string
326326
hosted: boolean
327-
}> {
327+
recommended?: boolean
328+
speedOptimized?: boolean
329+
deprecated?: boolean
330+
}
331+
332+
const DYNAMIC_PROVIDERS_NOTE = {
333+
note: 'The options array above lists Sim\'s static provider catalog. These providers also accept user-configured models that are NOT enumerated here: the user may have additional ids available at runtime (e.g. local Ollama tags). To reference one, prefix the model id with the provider slash below — for example "ollama/llama3.1:8b" instead of the bare "llama3.1:8b". The server rejects bare ids that are not in the catalog; always use the prefix for user-configured models.',
334+
prefixes: DYNAMIC_MODEL_PROVIDERS.map((p) => `${p}/`),
335+
} as const
336+
337+
function getStaticModelOptionsForVFS(): StaticModelOption[] {
328338
const hostedProviders = new Set(['openai', 'anthropic', 'google'])
329-
const dynamicProviders = new Set(['ollama', 'vllm', 'openrouter', 'fireworks'])
339+
const dynamicProviders = new Set<string>(DYNAMIC_MODEL_PROVIDERS)
330340

331-
const models: Array<{ id: string; provider: string; hosted: boolean }> = []
341+
const models: StaticModelOption[] = []
332342

333343
for (const [providerId, def] of Object.entries(PROVIDER_DEFINITIONS)) {
334344
if (dynamicProviders.has(providerId)) continue
335345
for (const model of def.models) {
336-
models.push({
346+
const option: StaticModelOption = {
337347
id: model.id,
338348
provider: providerId,
339349
hosted: hostedProviders.has(providerId),
340-
})
350+
}
351+
if (model.recommended) option.recommended = true
352+
if (model.speedOptimized) option.speedOptimized = true
353+
if (model.deprecated) option.deprecated = true
354+
models.push(option)
341355
}
342356
}
343357

@@ -378,9 +392,9 @@ export function serializeBlockSchema(block: BlockConfig): string {
378392
.map((sb) => {
379393
const serialized = serializeSubBlock(sb)
380394

381-
// For model comboboxes with function options, inject static model data with hosting info
382395
if (sb.id === 'model' && sb.type === 'combobox' && typeof sb.options === 'function') {
383396
serialized.options = getStaticModelOptionsForVFS()
397+
serialized.dynamicProviders = DYNAMIC_PROVIDERS_NOTE
384398
}
385399

386400
return serialized

0 commit comments

Comments
 (0)