mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-13 00:03:00 +00:00
fix: treat an authored empty effort override as no effort
This commit is contained in:
@@ -261,8 +261,10 @@ fn keep_authored_memory_id(
|
||||
}
|
||||
|
||||
/// A step's own `model` and `reasoning_effort` inputs replace the provider's, which is how a step
|
||||
/// linked to a saved agent picks them per run. An empty value keeps the provider's. A reasoning
|
||||
/// token is per model, so a different model without an effort of its own runs at its default.
|
||||
/// linked to a saved agent picks them per run. An empty model keeps the provider's, since a run
|
||||
/// needs one; an empty effort is no effort, which is how a model that reasons only on request is
|
||||
/// turned off. A reasoning token is per model, so a different model without an effort of its own
|
||||
/// runs at its default.
|
||||
/// Only an input the step authored counts: the arguments of an agent run as a tool also carry
|
||||
/// whatever the calling model sent, which must not choose the model it runs on.
|
||||
fn override_provider_model(
|
||||
@@ -270,7 +272,7 @@ fn override_provider_model(
|
||||
local_args: &HashMap<String, Box<RawValue>>,
|
||||
step_input_transforms: &HashMap<String, InputTransform>,
|
||||
) {
|
||||
let given = |key: &str| {
|
||||
let authored = |key: &str| {
|
||||
step_input_transforms
|
||||
.get(key)
|
||||
.filter(|t| {
|
||||
@@ -279,19 +281,22 @@ fn override_provider_model(
|
||||
InputTransform::Javascript { .. } | InputTransform::Static { .. }
|
||||
)
|
||||
})
|
||||
.and_then(|_| local_args.get(key))
|
||||
.and_then(|v| serde_json::from_str::<String>(v.get()).ok())
|
||||
.map(|s| s.trim().to_string())
|
||||
.filter(|s| !s.is_empty())
|
||||
.map(|_| {
|
||||
local_args
|
||||
.get(key)
|
||||
.and_then(|v| serde_json::from_str::<String>(v.get()).ok())
|
||||
.map(|s| s.trim().to_string())
|
||||
.unwrap_or_default()
|
||||
})
|
||||
};
|
||||
if let Some(model) = given("model") {
|
||||
if let Some(model) = authored("model").filter(|m| !m.is_empty()) {
|
||||
if model != provider.model {
|
||||
provider.model = model;
|
||||
provider.reasoning_effort = None;
|
||||
}
|
||||
}
|
||||
if let Some(effort) = given("reasoning_effort") {
|
||||
provider.reasoning_effort = Some(effort);
|
||||
if let Some(effort) = authored("reasoning_effort") {
|
||||
provider.reasoning_effort = Some(effort).filter(|e| !e.is_empty());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3012,8 +3017,13 @@ mod tests {
|
||||
assert_eq!(provider.model, "saved-model");
|
||||
|
||||
assert_eq!(run(json!({})), saved);
|
||||
assert_eq!(run(json!({ "model": "", "reasoning_effort": null })), saved);
|
||||
assert_eq!(run(json!({ "model": "" })), saved);
|
||||
assert_eq!(run(json!({ "model": "saved-model" })), saved);
|
||||
// An effort the step sets to nothing is no effort, not the saved one.
|
||||
assert_eq!(
|
||||
run(json!({ "reasoning_effort": null })),
|
||||
("saved-model".to_string(), None)
|
||||
);
|
||||
assert_eq!(
|
||||
run(json!({ "reasoning_effort": "low" })),
|
||||
("saved-model".to_string(), Some("low".to_string()))
|
||||
|
||||
Generated
+3
-3
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long
@@ -288,7 +288,7 @@ export const AGENT_FIELDS: AgentFieldSpec[] = [
|
||||
group: 'overrides',
|
||||
label: 'Reasoning effort',
|
||||
tooltip:
|
||||
'Runs this step at another reasoning effort than the one saved on the agent. Set it to an expression to choose the effort per run: an empty value keeps the saved effort. Overriding the model without it runs that model at its default effort.',
|
||||
'Runs this step at another reasoning effort than the one saved on the agent. Set it to an expression to choose the effort per run. An empty value sends no effort, so the model runs at its default. Overriding the model without it does the same.',
|
||||
implicit: '',
|
||||
defaultHint: 'Default: the effort saved on the agent'
|
||||
}
|
||||
|
||||
@@ -220,25 +220,28 @@ describe('providerWithOverrides', () => {
|
||||
})
|
||||
})
|
||||
|
||||
it('keeps an expression, falling back to the saved value when it is empty', () => {
|
||||
const folded = providerWithOverrides(saved, {
|
||||
reasoning_effort: { type: 'javascript', expr: 'flow_input.effort' }
|
||||
}) as { type: 'javascript'; expr: string }
|
||||
expect(folded.type).toBe('javascript')
|
||||
const run = (effort: unknown) => new Function('flow_input', `return ${folded.expr}`)({ effort })
|
||||
expect(run('low')).toEqual({ ...saved, reasoning_effort: 'low' })
|
||||
expect(run('')).toEqual(saved)
|
||||
it('keeps an expression, resolving as the linked step did', () => {
|
||||
const run = (overrides: Record<string, string>, flow_input: Record<string, unknown>) => {
|
||||
const folded = providerWithOverrides(
|
||||
saved,
|
||||
Object.fromEntries(
|
||||
Object.entries(overrides).map(([key, expr]) => [key, { type: 'javascript', expr }])
|
||||
)
|
||||
) as { type: 'javascript'; expr: string }
|
||||
expect(folded.type).toBe('javascript')
|
||||
return JSON.parse(
|
||||
JSON.stringify(new Function('flow_input', `return ${folded.expr}`)(flow_input))
|
||||
)
|
||||
}
|
||||
const other = { kind: 'openai', resource: '$res:u/a/openai', model: 'gpt-5-mini' }
|
||||
const effort = { reasoning_effort: 'flow_input.effort' }
|
||||
const model = { model: 'flow_input.model' }
|
||||
|
||||
const byModel = providerWithOverrides(saved, {
|
||||
model: { type: 'javascript', expr: 'flow_input.model' }
|
||||
}) as { type: 'javascript'; expr: string }
|
||||
const runModel = (model: unknown) =>
|
||||
JSON.parse(JSON.stringify(new Function('flow_input', `return ${byModel.expr}`)({ model })))
|
||||
expect(runModel('')).toEqual(saved)
|
||||
expect(runModel('gpt-5-mini')).toEqual({
|
||||
kind: 'openai',
|
||||
resource: '$res:u/a/openai',
|
||||
model: 'gpt-5-mini'
|
||||
})
|
||||
expect(run(effort, { effort: 'low' })).toEqual({ ...saved, reasoning_effort: 'low' })
|
||||
// An effort the step sets to nothing is no effort, not the saved one.
|
||||
expect(run(effort, { effort: '' })).toEqual({ ...other, model: 'gpt-5' })
|
||||
expect(run(model, { model: '' })).toEqual(saved)
|
||||
expect(run(model, { model: 'gpt-5-mini' })).toEqual(other)
|
||||
expect(run({ ...model, ...effort }, { model: 'gpt-5-mini', effort: '' })).toEqual(other)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -189,52 +189,42 @@ export function linkedAgentStepSchema(schema: any): any {
|
||||
|
||||
/**
|
||||
* The `provider` of a step unlinked from its agent: the saved one with the step's overrides folded
|
||||
* in, since an unlinked step sets model and effort nowhere else. Mirrors the worker: an empty
|
||||
* override keeps the saved value, and another model drops the saved effort.
|
||||
* in, since an unlinked step sets model and effort nowhere else. Follows the worker's
|
||||
* `override_provider_model`, so the step runs on what it ran on while linked.
|
||||
*/
|
||||
export function providerWithOverrides(
|
||||
provider: any,
|
||||
overrides: Partial<Record<AgentOverrideKey, InputTransform>> | undefined
|
||||
): InputTransform | undefined {
|
||||
if (!provider || typeof provider !== 'object') return undefined
|
||||
const set = (key: AgentOverrideKey): any => {
|
||||
const authored = (key: AgentOverrideKey): any => {
|
||||
const t = overrides?.[key] as any
|
||||
if (t?.type === 'javascript' && t.expr) return t
|
||||
if (t?.type === 'static' && typeof t.value === 'string' && t.value.trim()) return t
|
||||
return undefined
|
||||
}
|
||||
const model = set('model')
|
||||
const effort = set('reasoning_effort')
|
||||
const folded = { ...provider }
|
||||
if (model?.type === 'static' && model.value.trim() !== provider.model && !effort) {
|
||||
delete folded.reasoning_effort
|
||||
return t?.type === 'static' || (t?.type === 'javascript' && t.expr) ? t : undefined
|
||||
}
|
||||
const text = (t: any): string => (typeof t.value === 'string' ? t.value.trim() : '')
|
||||
const model = authored('model')
|
||||
const effort = authored('reasoning_effort')
|
||||
const { model: savedModel, reasoning_effort: savedEffort, ...rest } = provider
|
||||
|
||||
if (model?.type !== 'javascript' && effort?.type !== 'javascript') {
|
||||
if (model) folded.model = model.value.trim()
|
||||
if (effort) folded.reasoning_effort = effort.value.trim()
|
||||
return { type: 'static', value: folded }
|
||||
}
|
||||
const field = (key: AgentOverrideKey, t: any) => {
|
||||
if (!t && folded[key] === undefined) return []
|
||||
// The saved effort holds only while the model expression resolves to the saved model.
|
||||
if (!t && key === 'reasoning_effort' && model?.type === 'javascript') {
|
||||
const savedModel = JSON.stringify(folded.model)
|
||||
return [
|
||||
`${key}: ((${model.expr}) || ${savedModel}) === ${savedModel} ? ${JSON.stringify(folded[key])} : undefined`
|
||||
]
|
||||
const runModel = (model && text(model)) || savedModel
|
||||
const runEffort = effort ? text(effort) : runModel === savedModel ? savedEffort : undefined
|
||||
return {
|
||||
type: 'static',
|
||||
value: { ...rest, model: runModel, ...(runEffort ? { reasoning_effort: runEffort } : {}) }
|
||||
}
|
||||
if (!t) return [`${key}: ${JSON.stringify(folded[key])}`]
|
||||
if (t.type === 'static') return [`${key}: ${JSON.stringify(t.value.trim())}`]
|
||||
const saved = folded[key] === undefined ? 'undefined' : JSON.stringify(folded[key])
|
||||
return [`${key}: (${t.expr}) || ${saved}`]
|
||||
}
|
||||
const { model: _m, reasoning_effort: _e, ...rest } = folded
|
||||
const literal = (value: unknown) => (value === undefined ? 'undefined' : JSON.stringify(value))
|
||||
const source = (t: any) => (t.type === 'javascript' ? `(${t.expr})` : literal(text(t)))
|
||||
// One object literal, the shape the flow's expression evaluator and the chat composer both read.
|
||||
const runModel = model ? `(${source(model)} || ${literal(savedModel)})` : literal(savedModel)
|
||||
const runEffort = effort
|
||||
? `${source(effort)} || undefined`
|
||||
: `${runModel} === ${literal(savedModel)} ? ${literal(savedEffort)} : undefined`
|
||||
const fields = [
|
||||
...Object.entries(rest).map(
|
||||
([key, value]) => `${JSON.stringify(key)}: ${JSON.stringify(value)}`
|
||||
),
|
||||
...field('model', model),
|
||||
...field('reasoning_effort', effort)
|
||||
...Object.entries(rest).map(([key, value]) => `${JSON.stringify(key)}: ${literal(value)}`),
|
||||
`model: ${runModel}`,
|
||||
`reasoning_effort: ${runEffort}`
|
||||
]
|
||||
return { type: 'javascript', expr: `({ ${fields.join(', ')} })` }
|
||||
}
|
||||
|
||||
@@ -706,8 +706,11 @@
|
||||
<p class="mt-1 text-2xs text-hint">
|
||||
{#if spec.key === 'reasoning_effort' && effortCapability?.known && !effortCapability.supported}
|
||||
{effortModel} does not take a reasoning effort.
|
||||
{:else if spec.key === 'reasoning_effort'}
|
||||
Saved on the agent: {savedProvider.reasoning_effort || 'none'}. Left empty, the
|
||||
model runs at its default.
|
||||
{:else}
|
||||
Saved on the agent: {savedProvider[spec.key] || 'none'}
|
||||
Saved on the agent: {savedProvider.model || 'none'}
|
||||
{/if}
|
||||
</p>
|
||||
{/if}
|
||||
|
||||
@@ -528,9 +528,18 @@ describe('a step linked to a saved agent', () => {
|
||||
expect(agentModelWiringInputs(wiring).sort()).toEqual(['effort', 'model'])
|
||||
})
|
||||
|
||||
it('shows what the agent saved for an input left empty, the effort only on its own model', () => {
|
||||
it('shows the saved model for a model input left empty, and no effort for an empty effort', () => {
|
||||
const wiring = resolveAgentModelWiring([wired], saved)!
|
||||
expect(wiredFieldValue(wiring, {}, 'model')).toBe('claude-sonnet-4-5')
|
||||
// Empty is how a model that reasons only on request is turned off.
|
||||
expect(wiredFieldValue(wiring, { effort: '' }, 'reasoning_effort')).toBe('')
|
||||
})
|
||||
|
||||
it('shows the saved effort where the step sets none, only on the saved model', () => {
|
||||
const wiring = resolveAgentModelWiring(
|
||||
[linked({ model: { type: 'javascript', expr: 'flow_input.model' } })],
|
||||
saved
|
||||
)!
|
||||
expect(wiredFieldValue(wiring, {}, 'reasoning_effort')).toBe('high')
|
||||
expect(
|
||||
wiredFieldValue(wiring, { model: 'claude-haiku-4-5' }, 'reasoning_effort')
|
||||
|
||||
@@ -86,7 +86,8 @@ export type AgentModelWiring = {
|
||||
someAgentCannotRun?: boolean
|
||||
/**
|
||||
* What a step linked to a saved agent runs on where it overrides nothing: an empty model
|
||||
* input keeps the saved model, and the saved effort applies only on that model.
|
||||
* input keeps the saved model, and the saved effort applies only on that model, to a step
|
||||
* that sets no effort of its own.
|
||||
*/
|
||||
saved?: { model?: string; reasoning_effort?: string }
|
||||
}
|
||||
@@ -138,8 +139,8 @@ function parseLinkedStep(
|
||||
}
|
||||
|
||||
/**
|
||||
* The value a provider field runs on, given what the composer holds. Where the wiring is a
|
||||
* linked agent's, an empty value is the agent's saved one.
|
||||
* The value a provider field runs on, given what the composer holds, as the worker's
|
||||
* `override_provider_model` resolves a linked agent's.
|
||||
*/
|
||||
export function wiredFieldValue(
|
||||
wiring: AgentModelWiring,
|
||||
@@ -151,7 +152,8 @@ export function wiredFieldValue(
|
||||
const own = name ? values[name] : wiring.fixed[field]
|
||||
if (!wiring.saved || (own !== undefined && own !== '')) return own
|
||||
if (field === 'model') return wiring.saved.model
|
||||
if (field === 'reasoning_effort') {
|
||||
// An effort the step overrides is the run's whatever it holds: empty there is no effort.
|
||||
if (field === 'reasoning_effort' && !name) {
|
||||
return wiredFieldValue(wiring, values, 'model') === wiring.saved.model
|
||||
? wiring.saved.reasoning_effort
|
||||
: own
|
||||
|
||||
@@ -1197,9 +1197,9 @@ components:
|
||||
description: |
|
||||
String. Only on a step linked to a saved agent (see `agent`): replaces the reasoning
|
||||
effort saved on the agent for this step's runs, as a provider-native token (e.g.
|
||||
'low', 'high', 'none'). An empty value keeps the saved effort, unless `model`
|
||||
replaces the saved model, which then runs at its default effort. A step that is
|
||||
not linked sets both inside `provider`.
|
||||
'low', 'high', 'none'). An empty value sends no effort, so the model runs at its
|
||||
default. Leave the input out to keep the saved effort, which holds only while the
|
||||
step runs the saved model. A step that is not linked sets both inside `provider`.
|
||||
# Only the flow-local inputs are always present: a step linked to an `ai_agent` resource
|
||||
# (see `agent`) keeps just those and takes provider/output_type from the resource. Even
|
||||
# `user_message` may be absent, when memory is off and `previous_messages` is the prompt.
|
||||
|
||||
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long
@@ -151,8 +151,8 @@ needs becomes unreachable.
|
||||
is read instead as that one input carrying every field
|
||||
- A step linked to a saved agent (`agent` set) has no `provider` of its own. It exposes the same
|
||||
two controls through its `model` and `reasoning_effort` inputs, which replace what the agent
|
||||
saved: `"model": { "type": "javascript", "expr": "flow_input.model" }`. An empty value keeps the
|
||||
saved one, and the provider resource always stays the agent's
|
||||
saved: `"model": { "type": "javascript", "expr": "flow_input.model" }`. An empty model keeps the
|
||||
saved one, an empty effort sends none, and the provider resource always stays the agent's
|
||||
- `memory` is what lets the agent see earlier turns; without it every message starts from nothing
|
||||
- `streaming` on makes the answer and its thinking appear token by token instead of all at once
|
||||
- `user_attachments` points at a flow input typed as an array of s3 objects
|
||||
|
||||
Reference in New Issue
Block a user