Skip to content

Commit 2a7a055

Browse files
committed
improvement(agent): report a model level the sanitizer discards
A model bound to a variable or block reference only resolves at execution time, so a run whose reference landed on a model outside Sim's catalogue had its requested level cleared with no signal and quietly fell back to that model's default. Dropping stays the safe default — a provider with no such parameter rejects the whole request — but it is now reported. - log the field, model, and value whenever an unsupported-field level is cleared - cover both diagnostics, including that they stay quiet for a declared level and for the `auto` / `none` sentinels
1 parent 034aec7 commit 2a7a055

2 files changed

Lines changed: 120 additions & 6 deletions

File tree

apps/sim/providers/index.test.ts

Lines changed: 77 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,32 @@
44
import { envFlagsMockFns, resetEnvFlagsMock } from '@sim/testing'
55
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
66

7-
const { mockGetApiKeyWithBYOK, mockExecuteRequest } = vi.hoisted(() => ({
7+
const { mockGetApiKeyWithBYOK, mockExecuteRequest, mockLoggerWarn } = vi.hoisted(() => ({
88
mockGetApiKeyWithBYOK: vi.fn(),
99
mockExecuteRequest: vi.fn(),
10+
mockLoggerWarn: vi.fn(),
1011
}))
1112

13+
/** Overrides the global logger mock so the sanitizer's warnings are assertable. */
14+
vi.mock('@sim/logger', () => {
15+
const createLogger = () => ({
16+
info: vi.fn(),
17+
warn: mockLoggerWarn,
18+
error: vi.fn(),
19+
debug: vi.fn(),
20+
trace: vi.fn(),
21+
fatal: vi.fn(),
22+
child: () => createLogger(),
23+
withMetadata: () => createLogger(),
24+
})
25+
return {
26+
createLogger,
27+
logger: createLogger(),
28+
runWithRequestContext: <T>(_ctx: unknown, fn: () => T): T => fn(),
29+
getRequestContext: () => undefined,
30+
}
31+
})
32+
1233
vi.mock('@/lib/api-key/byok', () => ({
1334
getApiKeyWithBYOK: (...args: unknown[]) => mockGetApiKeyWithBYOK(...args),
1435
}))
@@ -528,4 +549,59 @@ describe('executeProviderRequest — model level normalization', () => {
528549
expect(sentRequest().reasoningEffort).toBeUndefined()
529550
expect(sentRequest().verbosity).toBeUndefined()
530551
})
552+
553+
/**
554+
* The model can itself be a reference, so it is only known at execution time. A run whose
555+
* reference resolved to a model outside Sim's catalogue must not fall back to that model's
556+
* default in silence.
557+
*/
558+
it('reports the level it drops when the resolved model does not support the field', async () => {
559+
await executeProviderRequest('anthropic', {
560+
model: 'claude-opus-4-6',
561+
workspaceId: 'ws-1',
562+
reasoningEffort: 'high',
563+
})
564+
565+
expect(mockLoggerWarn).toHaveBeenCalledWith(
566+
'Model does not support this level; dropping it from the request',
567+
expect.objectContaining({
568+
field: 'reasoningEffort',
569+
model: 'claude-opus-4-6',
570+
value: 'high',
571+
})
572+
)
573+
})
574+
575+
it('stays quiet when an unsupported model was never given a level', async () => {
576+
await executeProviderRequest('anthropic', {
577+
model: 'claude-opus-4-6',
578+
workspaceId: 'ws-1',
579+
})
580+
581+
expect(mockLoggerWarn).not.toHaveBeenCalled()
582+
})
583+
584+
it('reports a level the model accepts but does not declare', async () => {
585+
await executeProviderRequest('openai', {
586+
model: 'gpt-5',
587+
workspaceId: 'ws-1',
588+
reasoningEffort: 'xhigh',
589+
})
590+
591+
expect(mockLoggerWarn).toHaveBeenCalledWith(
592+
'Model level is not one this model declares; forwarding to the provider',
593+
expect.objectContaining({ field: 'reasoningEffort', model: 'gpt-5', value: 'xhigh' })
594+
)
595+
})
596+
597+
it('stays quiet for a declared level and for the auto and none sentinels', async () => {
598+
await executeProviderRequest('openai', {
599+
model: 'gpt-5',
600+
workspaceId: 'ws-1',
601+
reasoningEffort: 'auto',
602+
verbosity: 'high',
603+
})
604+
605+
expect(mockLoggerWarn).not.toHaveBeenCalled()
606+
})
531607
})

apps/sim/providers/index.ts

Lines changed: 43 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,34 @@ function normalizeModelLevel(value: string | undefined): string | undefined {
6464
*/
6565
const MODEL_LEVEL_SENTINELS = new Set(['auto', 'none'])
6666

67+
type ModelLevelField = 'reasoningEffort' | 'verbosity' | 'thinkingLevel'
68+
69+
/**
70+
* Clears a level whose resolved model does not accept the field at all.
71+
*
72+
* Dropping is the safe default — a provider that has no such parameter rejects the whole
73+
* request — but the discard is reported because the model can be bound to a variable or block
74+
* reference and is therefore only known at execution time. Without this, a run whose reference
75+
* resolved to a model outside Sim's catalogue would quietly fall back to that model's default
76+
* while the caller believed the level applied.
77+
*/
78+
function dropUnsupportedLevel(
79+
field: ModelLevelField,
80+
model: string,
81+
value: string | undefined
82+
): undefined {
83+
if (value) {
84+
logger.warn('Model does not support this level; dropping it from the request', {
85+
field,
86+
model,
87+
value,
88+
})
89+
}
90+
return undefined
91+
}
92+
6793
/**
68-
* Logs a level that is not one the model declares.
94+
* Logs a level that the model accepts as a field but does not list as a value.
6995
*
7096
* Deliberately does not drop the value. Sim's per-model level lists exist to populate the
7197
* pickers and can lag a provider that has started accepting a new level, so rejecting on them
@@ -74,7 +100,7 @@ const MODEL_LEVEL_SENTINELS = new Set(['auto', 'none'])
74100
* levels needs, where silently substituting the model default would corrupt the results.
75101
*/
76102
function warnOnUnrecognizedLevel(
77-
field: 'reasoningEffort' | 'verbosity' | 'thinkingLevel',
103+
field: ModelLevelField,
78104
model: string | undefined,
79105
value: string | undefined,
80106
declaredValues: string[] | null
@@ -103,15 +129,27 @@ function sanitizeRequest(request: ProviderRequest): ProviderRequest {
103129
}
104130

105131
if (model && !supportsReasoningEffort(model)) {
106-
sanitizedRequest.reasoningEffort = undefined
132+
sanitizedRequest.reasoningEffort = dropUnsupportedLevel(
133+
'reasoningEffort',
134+
model,
135+
sanitizedRequest.reasoningEffort
136+
)
107137
}
108138

109139
if (model && !supportsVerbosity(model)) {
110-
sanitizedRequest.verbosity = undefined
140+
sanitizedRequest.verbosity = dropUnsupportedLevel(
141+
'verbosity',
142+
model,
143+
sanitizedRequest.verbosity
144+
)
111145
}
112146

113147
if (model && !supportsThinking(model)) {
114-
sanitizedRequest.thinkingLevel = undefined
148+
sanitizedRequest.thinkingLevel = dropUnsupportedLevel(
149+
'thinkingLevel',
150+
model,
151+
sanitizedRequest.thinkingLevel
152+
)
115153
}
116154

117155
if (model && !supportsPromptCaching(model)) {

0 commit comments

Comments
 (0)